FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-121604: Raise DeprecationWarnings in importlib by rashansmith · Pull Request #121765 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-121604: Raise DeprecationWarnings in importlib #121765

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.

Sign up for GitHub

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

Closed
rashansmith wants to merge 7 commits into from
Conversation 27 Commits 7 Checks 0 Files changed
Closed

gh-121604: Raise DeprecationWarnings in importlib #121765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
03bbac0
Add deprecation warnings
rashansmith Jul 14, 2024
820ac21
📜🤖 Added by blurb_it.
blurb-it[bot] Jul 14, 2024
d6c0645
Update Lib/importlib/_abc.py
rashansmith Jul 14, 2024
cf8264d
Apply suggestions from code review
rashansmith Jul 14, 2024
cfbe079
Update machinery deprecation warning
rashansmith Jul 14, 2024
6294458
Apply suggestions from code review
rashansmith Jul 15, 2024
f2458cb
Merge branch 'main' into main
brettcannon Sep 26, 2024
File filter

Filter by extension

Filter by extension .py  (4) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
5 changes: 5 additions & 0 deletions Lib/importlib/_abc.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
"""Subset of importlib.abc used to reduce importlib.util imports."""
from . import _bootstrap
import abc
import warnings


class Loader(metaclass=abc.ABCMeta):

"""Abstract base class for import loaders."""
def __init__(self):
warnings.warn(f"Loader is deprecated.",
DeprecationWarning, stacklevel=2)
super().__init__()
Comment on lines +10 to +13

tomasr8 Jul 14, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not 100% sure but I think we should raise the deprecation warning when the class is imported, not when you create a new instance with it. Going by the example in the original PEP 562 it'd look (I think) something like this:

# Rename the deprecated class
class _Loader(metaclass=abc.ABCMeta):
    ...

def __getattr__(name):
    if name == 'Loader':
        warnings.warn(f"The 'Loader' class is deprecated.",
                      DeprecationWarning, stacklevel=2)
        return _Loader
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

rashansmith Jul 14, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, no problem. Thanks for the reference link! Another approach I had was to just put the warning message by itself in the first line under the class name. Would this have a similar affect? I removed it because I then saw the deprecation error when running the make command.

tomasr8 Jul 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think the problem with the other approach is that you'll get the warning whenever you import the module, not just the deprecated class specifically. For example, import importlib.abc will produce a warning for InspectLoader even if you're not actually using it

def create_module(self, spec):
"""Return a module to initialize and into which to load.
Expand Down
11 changes: 11 additions & 0 deletions Lib/importlib/_bootstrap_external.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,10 @@ def path_mtime(self, path):

Raises OSError when the path cannot be handled.
"""
_warnings.warn(
f"SourcelessFileLoader is deprecated.",
DeprecationWarning,
stacklevel=2)
raise OSError

def path_stats(self, path):
Expand Down Expand Up @@ -1000,6 +1004,13 @@ def set_data(self, path, data, *, _mode=0o666):
class SourcelessFileLoader(FileLoader, _LoaderBasics):

"""Loader which handles sourceless file imports."""
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
_warnings.warn(
f"SourcelessFileLoader is deprecated.",
DeprecationWarning,
stacklevel=2
)

def get_code(self, fullname):
path = self.get_filename(fullname)
Expand Down
13 changes: 13 additions & 0 deletions Lib/importlib/abc.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
_frozen_importlib_external = _bootstrap_external
from ._abc import Loader
import abc
import warnings


__all__ = [
Expand All @@ -22,6 +23,11 @@
]


def __getattr__(name):
if name == 'ResourceLoader':
warnings.warn(f"The '{name}' attribute is deprecated.",
DeprecationWarning, stacklevel=2)

def _register(abstract_cls, *classes):
for cls in classes:
abstract_cls.register(cls)
Expand Down Expand Up @@ -69,6 +75,8 @@ class ResourceLoader(Loader):
This ABC represents one of the optional protocols specified by PEP 302.

"""
warnings.warn(f"The Resource Loader class is deprecated.",
DeprecationWarning, stacklevel=2)

@abc.abstractmethod
def get_data(self, path):
Expand All @@ -85,6 +93,8 @@ class InspectLoader(Loader):
This ABC represents one of the optional protocols specified by PEP 302.

"""
warnings.warn(f"The InspectLoader class is deprecated.",
DeprecationWarning, stacklevel=2)

def is_package(self, fullname):
"""Optional method which when implemented should return whether the
Expand Down Expand Up @@ -198,6 +208,9 @@ class SourceLoader(_bootstrap_external.SourceLoader, ResourceLoader, ExecutionLo
"""

def path_mtime(self, path):
"""Deprecated"""
warnings.warn(f"The path_mtime function is deprecated.",
DeprecationWarning, stacklevel=2)
"""Return the (int) modification time for the path (str)."""
if self.path_stats.__func__ is SourceLoader.path_stats:
raise OSError
Expand Down
13 changes: 13 additions & 0 deletions Lib/importlib/machinery.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""The machinery of importlib: finders, loaders, hooks, etc."""

tomasr8 Jul 14, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd keep the newline
import warnings
from ._bootstrap import ModuleSpec
from ._bootstrap import BuiltinImporter
from ._bootstrap import FrozenImporter
Expand Down Expand Up @@ -27,3 +28,15 @@ def all_suffixes():
'NamespaceLoader', 'OPTIMIZED_BYTECODE_SUFFIXES', 'PathFinder',
'SOURCE_SUFFIXES', 'SourceFileLoader', 'SourcelessFileLoader',
'WindowsRegistryFinder', 'all_suffixes']


def __getattr__(name):
if name in ('DEBUG_BYTECODE_SUFFIXES', 'OPTIMIZED_BYTECODE_SUFFIXES', 'WindowsRegistryFinder'):

CharlieZhao95 Jul 15, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The first if statement could be omitted, just use the if-elif statement ;)

if name in ('DEBUG_BYTECODE_SUFFIXES', 'OPTIMIZED_BYTECODE_SUFFIXES'):
    ...
elif name == 'WindowsRegistryFinder':
    ...
if name in ('DEBUG_BYTECODE_SUFFIXES', 'OPTIMIZED_BYTECODE_SUFFIXES'):
warnings.warn(f"The '{name}' module is deprecated.",
DeprecationWarning, stacklevel=2)
return name
else:
warnings.warn(f"The '{name}' class is deprecated.",
DeprecationWarning, stacklevel=2)
return name
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2024-07-14-12-46-02.gh-issue-121754.2Tt2fE.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This PR adds some deprecation warnings to importlib
Back | FazBrowse Home | New Git URL