Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
bpo-29576: Improve some deprecations in the importlib#32
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Lib/importlib/abc.py Outdated
| """ | ||
| warnings.warn("MetaPathFinder.find_module is deprecated since Python " | ||
| "3.4 in favor of MetaPatFinder.find_spec " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
typo here s/MetaPatFinder/MetaPathFinder/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oops indeed, thanks.
Codecov Report
@@ Coverage Diff @@## master #32 +/- ##==========================================- Coverage 82.37% 82.36% -0.01%========================================== Files 1427 1427 Lines 350948 350951 +3 ==========================================- Hits 289093 289062 -31- Misses 61855 61889 +34 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could you add tests to make sure the warnings are being raised?
Lib/importlib/abc.py Outdated
| @@ -97,7 +106,13 @@ def find_loader(self, fullname): | |||
| This method is deprecated in favor of finder.find_spec(). If find_spec() | |||
| is provided than backwards-compatible functionality is provided. | |||
| Deprecated since Python 3.4 in favor of find_spec (available since 3.4) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This line is somewhat redundant since the deprecation is mentioned in the previous paragraph. Maybe just add the version it was deprecated in above?
Lib/importlib/abc.py Outdated
| @@ -61,7 +64,13 @@ def find_module(self, fullname, path): | |||
| exists then backwards-compatible functionality is provided for this | |||
| method. | |||
| Deprecated since Python 3.4 in favor of find_spec (available since 3.4) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The deprecation is mentioned in the above paragraph, so if you would like to add the version info just toss in up there.
Lib/importlib/abc.py Outdated
| """ | ||
| warnings.warn("MetaPathFinder.find_module is deprecated since Python " | ||
| "3.4 in favor of MetaPathFinder.find_spec " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please add() afterfind_spec.
Lib/importlib/abc.py Outdated
| """ | ||
| warnings.warn("PathEntryFinder.find_loader is deprecated since Python " | ||
| "3.4 in favor of PathEntryFinder.find_spec " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please add() as appropriate.
Add the python version since the functionality is deprecated,and raise a couple of deprecation warnings in a few places.Theses functions are marked as deprecated in the documentation, butespecially in existing codebase, programmers tends to not re-checkwhether functions are deprecated. So trigger the warning when possible.It's also more probable that a developer will drop deprecatedfunctionality if we immediately give them information aboutreplacement API, and not have them to go find it in the documentation.Include deprecation information in DocString as well as many tools pulldocumentation from there and not from docs.python.org.Add test making sure `find_loader()` and `find_module()` Both emit adeprecation warning.
All comments should be addressed. |
Thanks! Please add entries in Misc/NEWS and one in What's New in the "deprecated" section and this will be ready to be merged! |
Thanks! |
Thanks ! |
Add the python version since the functionality is deprecated,
and raise a couple of deprecation warnings in a few places.
Theses functions are marked as deprecated in the documentation, but
especially in existing codebase, programmers tends to not re-check
whether functions are deprecated. So trigger the warning when possible.
It's also more probable that a developer will drop deprecated
functionality if we immediately give them information about
replacement API, and not have them to go find it in the documentation.
Include deprecation information in DocString as well, as many tools pull
documentation from there and not from docs.python.org.