Skip to content

bpo-42137: have ModuleType.__repr__ prefer __spec__ over module_repr() #24953

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

Merged
merged 11 commits into from
Mar 24, 2021
14 changes: 7 additions & 7 deletions Doc/library/importlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Functions
.. versionadded:: 3.4
.. versionchanged:: 3.7
:exc:`ModuleNotFoundError` is raised when the module being reloaded lacks
a :class:`ModuleSpec`.
a :class:`~importlib.machinery.ModuleSpec`.


:mod:`importlib.abc` -- Abstract base classes related to import
Expand Down Expand Up @@ -1591,19 +1591,19 @@ an :term:`importer`.

.. function:: spec_from_loader(name, loader, *, origin=None, is_package=None)

A factory function for creating a :class:`ModuleSpec` instance based
on a loader. The parameters have the same meaning as they do for
ModuleSpec. The function uses available :term:`loader` APIs, such as
A factory function for creating a :class:`~importlib.machinery.ModuleSpec`
instance based on a loader. The parameters have the same meaning as they do
for ModuleSpec. The function uses available :term:`loader` APIs, such as
:meth:`InspectLoader.is_package`, to fill in any missing
information on the spec.

.. versionadded:: 3.4

.. function:: spec_from_file_location(name, location, *, loader=None, submodule_search_locations=None)

A factory function for creating a :class:`ModuleSpec` instance based
on the path to a file. Missing information will be filled in on the
spec by making use of loader APIs and by the implication that the
A factory function for creating a :class:`~importlib.machinery.ModuleSpec`
instance based on the path to a file. Missing information will be filled in
on the spec by making use of loader APIs and by the implication that the
module will be file-based.

.. versionadded:: 3.4
Expand Down
6 changes: 6 additions & 0 deletions Doc/whatsnew/3.10.rst
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,12 @@ Deprecated
:meth:`~importlib.abc.Loader.exec_module` is preferred.
(Contributed by Brett Cannon in :issue:`26131`.)

* The import system now uses the ``__spec__`` attribute on modules before
falling back on :meth:`~importlib.abc.Loader.module_repr` for a module's
``__repr__()`` method. Removal of the use of ``module_repr()`` is scheduled
for Python 3.12.
(Contributed by Brett Cannon in :issue:`42137`.)

* ``sqlite3.OptimizedUnicode`` has been undocumented and obsolete since Python
3.3, when it was made an alias to :class:`str`. It is now deprecated,
scheduled for removal in Python 3.12.
Expand Down
20 changes: 6 additions & 14 deletions Lib/importlib/_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ def _requires_frozen_wrapper(self, fullname):
def _load_module_shim(self, fullname):
"""Load the specified module into sys.modules and return it.

This method is deprecated. Use loader.exec_module instead.
This method is deprecated. Use loader.exec_module() instead.

"""
msg = ("the load_module() method is deprecated and slated for removal in "
Expand All @@ -292,24 +292,16 @@ def _load_module_shim(self, fullname):
# Module specifications #######################################################

def _module_repr(module):
# The implementation of ModuleType.__repr__().
"""The implementation of ModuleType.__repr__()."""
loader = getattr(module, '__loader__', None)
if hasattr(loader, 'module_repr'):
# As soon as BuiltinImporter, FrozenImporter, and NamespaceLoader
# drop their implementations for module_repr. we can add a
# deprecation warning here.
if spec := getattr(module, "__spec__", None):
return _module_repr_from_spec(spec)
elif hasattr(loader, 'module_repr'):
try:
return loader.module_repr(module)
except Exception:
pass
try:
spec = module.__spec__
except AttributeError:
pass
else:
if spec is not None:
return _module_repr_from_spec(spec)

# Fall through to a catch-all which always succeeds.
# We could use module.__class__.__name__ instead of 'module' in the
# various repr permutations.
try:
Expand Down
8 changes: 0 additions & 8 deletions Lib/test/test_importlib/frozen/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,6 @@ def test_module_repr(self):
self.assertEqual(repr_str,
"<module '__hello__' (frozen)>")

def test_module_repr_indirect(self):
with warnings.catch_warnings():
warnings.simplefilter("ignore", DeprecationWarning)
with util.uncache('__hello__'), captured_stdout():
module = self.machinery.FrozenImporter.load_module('__hello__')
self.assertEqual(repr(module),
"<module '__hello__' (frozen)>")

# No way to trigger an error in a frozen module.
test_state_after_failure = None

Expand Down
3 changes: 2 additions & 1 deletion Lib/test/test_importlib/test_namespace_pkgs.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ def test_cant_import_other(self):

def test_module_repr(self):
import foo.one
self.assertEqual(repr(foo), "<module 'foo' (namespace)>")
self.assertEqual(foo.__spec__.loader.module_repr(foo),
"<module 'foo' (namespace)>")


class DynamicPathNamespacePackage(NamespacePackageTest):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The import system now prefers using ``__spec__`` for ``ModuleType.__repr__``
over ``module_repr()``.
Loading