Guard against import machinery masking out attributes#171
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
===========================================
- Coverage 100.00% 97.54% -2.46%
===========================================
Files 1 1
Lines 109 122 +13
Branches 19 23 +4
===========================================
+ Hits 109 119 +10
- Misses 0 1 +1
- Partials 0 2 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
effigies
left a comment
There was a problem hiding this comment.
Well that's a gnarly bug. This fix definitely looks right. One question:
| # | ||
| # Record affected cases and, only in those cases, swap in the | ||
| # guarding module type. | ||
| shadowed = {attr for attr, mod in attr_to_modules.items() if attr == mod} |
There was a problem hiding this comment.
Do we need to worry about submodules? This handles the case of from .x import x, but what about from .w.x import x or something like that?
There was a problem hiding this comment.
Or from .x.w import x 👀 I suspect from .w.x import x should be OK.
There was a problem hiding this comment.
Great catch, thanks for that!
|
OK, hopefully the last commit does the trick! Once we are confident that this works, we should probably tag a new release (at least skimage needs it). |
Thank you to @matthew-brett for uncovering and analyzing this issue.
This deals with the case where a function
my_funcis defined inmy_func.py. In some cases where themy_funcsubmodule is imported (e.g.from pkg.my_func import ...,import pkg.my_func, orimport *accessing a sibling attribute in the same file) the import machinery updates the package__dict__with themy_func(the submodule) overriding access tomy_func(the function).This wraps the package
__setattr__to intercept those (and only those) problematic assignments.