Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.4k
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
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.
|
brettcannon left a comment
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
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
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
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
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.
Carreau commentedFeb 15, 2017
All comments should be addressed. |
brettcannon commentedFeb 15, 2017
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! |
brettcannon commentedFeb 16, 2017
Thanks! |
Carreau commentedFeb 16, 2017
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.