-
Notifications
You must be signed in to change notification settings - Fork 167
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
correct identification of stdlib TypeVar objs #412
Conversation
In Python <= 3.7, the module attribute of TypeVar constructs is always set to typing and thus irrelevant to the true origin of the construct (stdlib, third-party, user-defined). cloudpickle used to cirumvent this issue in `_whichmodule` by exhaustively search for the construct in all imported modules. This would generate false positive in the case of stdlib TypeVar constructs such as AnyStr being used by imported third-party module. For such TypeVar construct, `_whichmodule` would occasionally flag them as belonging to the third party module. This resulted in a flaky test (`test_lookup_module_and_qualname_stdlib_typevar`) where `typing.AnyStr` was occasionally marked as defined in `_pytest.capture` instead of `typing`.
Codecov Report
@@ Coverage Diff @@
## master #412 +/- ##
==========================================
+ Coverage 91.61% 91.70% +0.08%
==========================================
Files 3 4 +1
Lines 656 663 +7
Branches 135 136 +1
==========================================
+ Hits 601 608 +7
Misses 34 34
Partials 21 21
Continue to review full report at Codecov.
|
(credits to @faucct - cloudpipe#411)
cc @ogrisel if you want to take a quick look. |
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.
LGTM besides question below:
Also please document the fix in the changelog. |
Merged. Thanks @pierreglaser! |
In Python <= 3.7, the module attribute of
TypeVar
constructs is always set to typing and thus irrelevant to the true origin of the construct (stdlib, third-party, user-defined).cloudpickle
used to cirumvent this issue in_whichmodule
by exhaustively search for the construct in all imported modules.This can generate false positives in the case of stdlib
TypeVar
constructs such asAnyStr
being used by a imported third-party module. For suchTypeVar
constructs,_whichmodule
would occasionally flag them as belonging to the said third party module.This resulted in a flaky test (
test_lookup_module_and_qualname_stdlib_typevar
) wheretyping.AnyStr
was occasionally marked as defined in_pytest.capture
instead oftyping
.This PR fixes this bug by looking if the
TypeVar
construct is defined intyping
before triggering a search insys.modules
.