correct identification of stdlib TypeVar objs #412
Merged
Conversation
This file contains 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
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. |
ogrisel
approved these changes
Feb 27, 2021
|
Also please document the fix in the changelog. |
ogrisel
reviewed
Mar 1, 2021
|
Merged. Thanks @pierreglaser! |
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.
In Python <= 3.7, the module attribute of
TypeVarconstructs is always set to typing and thus irrelevant to the true origin of the construct (stdlib, third-party, user-defined).cloudpickleused to cirumvent this issue in_whichmoduleby exhaustively search for the construct in all imported modules.This can generate false positives in the case of stdlib
TypeVarconstructs such asAnyStrbeing used by a imported third-party module. For suchTypeVarconstructs,_whichmodulewould 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.AnyStrwas occasionally marked as defined in_pytest.captureinstead oftyping.This PR fixes this bug by looking if the
TypeVarconstruct is defined intypingbefore triggering a search insys.modules.