[MRG] MNT Uses pep562 to import private methods #15431
Merged
+75
−2
Conversation
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
|
I think this is missing import sys
PY37 = sys.version_info >= (3, 7)
if not PY37:
Pep562(__name__) |
NicolasHug
reviewed
Nov 1, 2019
| @@ -143,7 +148,9 @@ def test_tabs(): | |||
| 'feature_extraction._hashing_fast' in modname): | |||
| continue | |||
|
|
|||
| # because we don't import | |||
| # Do not check deprecated paths | |||
| if modname in DEPRECATED_MODULE_NAMES: | |||
There was a problem hiding this comment.
why do we need this? There shouldn't be any tab anyway?
There was a problem hiding this comment.
The Pep562 wraps the original module and is an instance, which means getsource will fail in this case.
I updated this PR by unwrapping the instance and grab the underlying _module.
| @@ -133,6 +134,10 @@ def test_docstring_parameters(): | |||
| raise AssertionError("Docstring Error:\n" + msg) | |||
|
|
|||
|
|
|||
| DEPRECATED_MODULE_NAMES = set( | |||
There was a problem hiding this comment.
declare in the function where it's used?
jnothman
approved these changes
Nov 2, 2019
NicolasHug
reviewed
Nov 2, 2019
| @@ -145,6 +146,12 @@ def test_tabs(): | |||
|
|
|||
| # because we don't import | |||
| mod = importlib.import_module(modname) | |||
|
|
|||
| # unwrap to get module because Pep562 backport wraps the original | |||
| # module | |||
There was a problem hiding this comment.
Mark as to be removed in 0.24 or when Pep562 isn't used anymore
?
There was a problem hiding this comment.
We can also used this for experimental features for a better error message?
NicolasHug
approved these changes
Nov 2, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Resolves #15415
Alternative to #15419
What does this implement/fix? Explain your changes.
Lets backport pep562, since we may be using it for
enable_experimental_*as well.