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 4 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
15 changes: 12 additions & 3 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 @@ -554,7 +553,7 @@ the module.

.. 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__``
Expand All @@ -570,6 +569,16 @@ the module.
The value of ``__package__`` is expected to be the same as
``__spec__.parent``.

.. versionchanged:: 3.10
:exc:`ImportWarning` is raised if import must fall back to
brettcannon marked this conversation as resolved.
Show resolved Hide resolved
``__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 +587,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
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``.
brettcannon marked this conversation as resolved.
Show resolved Hide resolved

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