Skip to content

Commit

Permalink
gh-65961: Raise DeprecationWarning when __package__ differs from …
Browse files Browse the repository at this point in the history
…`__spec__.parent` (#97879)

Also remove `importlib.util.set_package()` which was already slated for removal.

Co-authored-by: Eric Snow <[email protected]>
  • Loading branch information
brettcannon and ericsnowcurrently authored Oct 5, 2022
1 parent 2016bc5 commit c206e53
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 102 deletions.
9 changes: 0 additions & 9 deletions Doc/library/importlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
30 changes: 24 additions & 6 deletions Doc/reference/import.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -552,24 +555,36 @@ 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__``
should be set to the empty string for top-level modules, or for
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
Expand All @@ -578,7 +593,7 @@ the module.
interpreter startup <programs>`. The one exception is ``__main__``,
where ``__spec__`` is :ref:`set to None in some cases <main_spec>`.

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
Expand Down Expand Up @@ -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__
Expand Down
12 changes: 12 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------------------------
Expand Down Expand Up @@ -275,6 +280,9 @@ Pending Removal in Python 3.14
* Creating :c:data:`immutable types <Py_TPFLAGS_IMMUTABLETYPE>` 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
----------------------------------
Expand Down Expand Up @@ -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
======================

Expand Down
2 changes: 1 addition & 1 deletion Lib/importlib/_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 0 additions & 20 deletions Lib/importlib/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_importlib/import_/test___package__.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ def test_spec_fallback(self):
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')})

Expand Down
63 changes: 0 additions & 63 deletions Lib/test/test_importlib/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,69 +252,6 @@ def load_module(self, module):
) = 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__ = ['<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()."""
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit c206e53

Please sign in to comment.