Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-65961: Raise DeprecationWarning when __package__ differs from __spec__.parent #97879

Merged
merged 6 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
brettcannon marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -206,6 +206,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 @@ -266,6 +271,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 @@ -418,6 +426,10 @@ Removed
(Contributed by Victor Stinner in :gh:`94199`.)


* ``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 @@ -1250,7 +1250,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