From c206e53bb726fa795d10cfb0e8d1d1a1a5d1aaa7 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 5 Oct 2022 15:00:45 -0700 Subject: [PATCH] gh-65961: Raise `DeprecationWarning` when `__package__` differs from `__spec__.parent` (#97879) Also remove `importlib.util.set_package()` which was already slated for removal. Co-authored-by: Eric Snow --- Doc/library/importlib.rst | 9 --- Doc/reference/import.rst | 30 +++++++-- Doc/whatsnew/3.12.rst | 12 ++++ Lib/importlib/_bootstrap.py | 2 +- Lib/importlib/util.py | 20 ------ .../import_/test___package__.py | 4 +- Lib/test/test_importlib/test_util.py | 63 ------------------- ...2-10-05-00-37-27.gh-issue-65961.z0Ys0y.rst | 5 ++ Python/import.c | 2 +- 9 files changed, 45 insertions(+), 102 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-10-05-00-37-27.gh-issue-65961.z0Ys0y.rst diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 0fd765f5985..a7c067c06e8 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -1378,15 +1378,6 @@ an :term:`importer`. .. deprecated:: 3.4 The import machinery takes care of this automatically. -.. decorator:: set_package - - A :term:`decorator` for :meth:`importlib.abc.Loader.load_module` to set the - :attr:`__package__` attribute on the returned module. If :attr:`__package__` - is set and has a value other than ``None`` it will not be changed. - - .. deprecated:: 3.4 - The import machinery takes care of this automatically. - .. function:: spec_from_loader(name, loader, *, origin=None, is_package=None) A factory function for creating a :class:`~importlib.machinery.ModuleSpec` diff --git a/Doc/reference/import.rst b/Doc/reference/import.rst index 507f2b3763c..58f4ef897bd 100644 --- a/Doc/reference/import.rst +++ b/Doc/reference/import.rst @@ -358,7 +358,6 @@ of what happens during the loading portion of import:: sys.modules[spec.name] = module elif not hasattr(spec.loader, 'exec_module'): module = spec.loader.load_module(spec.name) - # Set __loader__ and __package__ if missing. else: sys.modules[spec.name] = module try: @@ -539,6 +538,10 @@ The import machinery fills in these attributes on each module object during loading, based on the module's spec, before the loader executes the module. +It is **strongly** recommended that you rely on :attr:`__spec__` and +its attributes instead of any of the other individual attributes +listed below. + .. attribute:: __name__ The ``__name__`` attribute must be set to the fully qualified name of @@ -552,9 +555,12 @@ the module. for introspection, but can be used for additional loader-specific functionality, for example getting data associated with a loader. + It is **strongly** recommended that you rely on :attr:`__spec__` + instead instead of this attribute. + .. attribute:: __package__ - The module's ``__package__`` attribute must be set. Its value must + The module's ``__package__`` attribute may be set. Its value must be a string, but it can be the same value as its ``__name__``. When the module is a package, its ``__package__`` value should be set to its ``__name__``. When the module is not a package, ``__package__`` @@ -562,14 +568,23 @@ the module. submodules, to the parent package's name. See :pep:`366` for further details. - This attribute is used instead of ``__name__`` to calculate explicit - relative imports for main modules, as defined in :pep:`366`. It is - expected to have the same value as ``__spec__.parent``. + It is **strongly** recommended that you rely on :attr:`__spec__` + instead instead of this attribute. .. versionchanged:: 3.6 The value of ``__package__`` is expected to be the same as ``__spec__.parent``. + .. versionchanged:: 3.10 + :exc:`ImportWarning` is raised if import falls back to + ``__package__`` instead of + :attr:`~importlib.machinery.ModuleSpec.parent`. + + .. versionchanged:: 3.12 + Raise :exc:`DeprecationWarning` instead of :exc:`ImportWarning` + when falling back to ``__package__``. + + .. attribute:: __spec__ The ``__spec__`` attribute must be set to the module spec that was @@ -578,7 +593,7 @@ the module. interpreter startup `. The one exception is ``__main__``, where ``__spec__`` is :ref:`set to None in some cases `. - When ``__package__`` is not defined, ``__spec__.parent`` is used as + When ``__spec__.parent`` is not set, ``__package__`` is used as a fallback. .. versionadded:: 3.4 @@ -623,6 +638,9 @@ the module. if a loader can load from a cached module but otherwise does not load from a file, that atypical scenario may be appropriate. + It is **strongly** recommended that you rely on :attr:`__spec__` + instead instead of ``__cached__``. + .. _package-path-rules: module.__path__ diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 2e9515d036e..507ba352214 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -215,6 +215,11 @@ Deprecated may be removed in a future version of Python. Use the single-arg versions of these functions instead. (Contributed by Ofey Chan in :gh:`89874`.) +* :exc:`DeprecationWarning` is now raised when ``__package__`` on a + module differs from ``__spec__.parent`` (previously it was + :exc:`ImportWarning`). + (Contributed by Brett Cannon in :gh:`65961`.) + Pending Removal in Python 3.13 ------------------------------ @@ -275,6 +280,9 @@ Pending Removal in Python 3.14 * Creating :c:data:`immutable types ` with mutable bases using the C API. +* ``__package__`` will cease to be set or taken into consideration by + the import system (:gh:`97879`). + Pending Removal in Future Versions ---------------------------------- @@ -432,6 +440,10 @@ Removed * References to, and support for ``module_repr()`` has been eradicated. +* ``importlib.util.set_package`` has been removed. + (Contributed by Brett Cannon in :gh:`65961`.) + + Porting to Python 3.12 ====================== diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 5d3c9fe3fbd..1c132106ce5 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -1228,7 +1228,7 @@ def _calc___package__(globals): if spec is not None and package != spec.parent: _warnings.warn("__package__ != __spec__.parent " f"({package!r} != {spec.parent!r})", - ImportWarning, stacklevel=3) + DeprecationWarning, stacklevel=3) return package elif spec is not None: return spec.parent diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 8623c89840c..7f15b029b24 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -141,26 +141,6 @@ def _module_to_load(name): module.__initializing__ = False -def set_package(fxn): - """Set __package__ on the returned module. - - This function is deprecated. - - """ - @functools.wraps(fxn) - def set_package_wrapper(*args, **kwargs): - warnings.warn('The import system now takes care of this automatically; ' - 'this decorator is slated for removal in Python 3.12', - DeprecationWarning, stacklevel=2) - module = fxn(*args, **kwargs) - if getattr(module, '__package__', None) is None: - module.__package__ = module.__name__ - if not hasattr(module, '__path__'): - module.__package__ = module.__package__.rpartition('.')[0] - return module - return set_package_wrapper - - def set_loader(fxn): """Set __loader__ on the returned module. diff --git a/Lib/test/test_importlib/import_/test___package__.py b/Lib/test/test_importlib/import_/test___package__.py index 1ab5018a431..ab1b35ee3c1 100644 --- a/Lib/test/test_importlib/import_/test___package__.py +++ b/Lib/test/test_importlib/import_/test___package__.py @@ -74,8 +74,8 @@ class Using__package__: self.assertEqual(module.__name__, 'pkg') def test_warn_when_package_and_spec_disagree(self): - # Raise an ImportWarning if __package__ != __spec__.parent. - with self.assertWarns(ImportWarning): + # Raise a DeprecationWarning if __package__ != __spec__.parent. + with self.assertWarns(DeprecationWarning): self.import_module({'__package__': 'pkg.fake', '__spec__': FakeSpec('pkg.fakefake')}) diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index a62d68fcd8b..e70971e9d3b 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -252,69 +252,6 @@ class ModuleForLoaderTests: ) = util.test_both(ModuleForLoaderTests, util=importlib_util) -class SetPackageTests: - - """Tests for importlib.util.set_package.""" - - def verify(self, module, expect): - """Verify the module has the expected value for __package__ after - passing through set_package.""" - fxn = lambda: module - wrapped = self.util.set_package(fxn) - with warnings.catch_warnings(): - warnings.simplefilter('ignore', DeprecationWarning) - wrapped() - self.assertTrue(hasattr(module, '__package__')) - self.assertEqual(expect, module.__package__) - - def test_top_level(self): - # __package__ should be set to the empty string if a top-level module. - # Implicitly tests when package is set to None. - module = types.ModuleType('module') - module.__package__ = None - self.verify(module, '') - - def test_package(self): - # Test setting __package__ for a package. - module = types.ModuleType('pkg') - module.__path__ = [''] - module.__package__ = None - self.verify(module, 'pkg') - - def test_submodule(self): - # Test __package__ for a module in a package. - module = types.ModuleType('pkg.mod') - module.__package__ = None - self.verify(module, 'pkg') - - def test_setting_if_missing(self): - # __package__ should be set if it is missing. - module = types.ModuleType('mod') - if hasattr(module, '__package__'): - delattr(module, '__package__') - self.verify(module, '') - - def test_leaving_alone(self): - # If __package__ is set and not None then leave it alone. - for value in (True, False): - module = types.ModuleType('mod') - module.__package__ = value - self.verify(module, value) - - def test_decorator_attrs(self): - def fxn(module): pass - with warnings.catch_warnings(): - warnings.simplefilter('ignore', DeprecationWarning) - wrapped = self.util.set_package(fxn) - self.assertEqual(wrapped.__name__, fxn.__name__) - self.assertEqual(wrapped.__qualname__, fxn.__qualname__) - - -(Frozen_SetPackageTests, - Source_SetPackageTests - ) = util.test_both(SetPackageTests, util=importlib_util) - - class SetLoaderTests: """Tests importlib.util.set_loader().""" diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-10-05-00-37-27.gh-issue-65961.z0Ys0y.rst b/Misc/NEWS.d/next/Core and Builtins/2022-10-05-00-37-27.gh-issue-65961.z0Ys0y.rst new file mode 100644 index 00000000000..0c034263c1a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-10-05-00-37-27.gh-issue-65961.z0Ys0y.rst @@ -0,0 +1,5 @@ +When ``__package__`` is different than ``__spec__.parent``, raise a +``DeprecationWarning`` instead of ``ImportWarning``. + +Also remove ``importlib.util.set_package()`` which was scheduled for +removal. diff --git a/Python/import.c b/Python/import.c index 54c21fa4a56..698ef37ce0a 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1573,7 +1573,7 @@ resolve_name(PyThreadState *tstate, PyObject *name, PyObject *globals, int level goto error; } else if (equal == 0) { - if (PyErr_WarnEx(PyExc_ImportWarning, + if (PyErr_WarnEx(PyExc_DeprecationWarning, "__package__ != __spec__.parent", 1) < 0) { goto error; }