-
-
Notifications
You must be signed in to change notification settings - Fork 313
Fix decorator frame #2901
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
Fix decorator frame #2901
Conversation
| assert module.locals.get("x") == [name_expr_node1.target] | ||
| assert module.locals.get("y") == [name_expr_node2.target] |
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 frame is used for the scope lookup to determine where to assign the variables to. Without the changes in this PR, any variables get assigned to the function / class scope instead even though decorators are evaluated in the module scope.
--
A similar workaround might be required for function argument defaults. Though I haven't searched the issues tracker to see if anyone has complained yet.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (75.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2901 +/- ##
==========================================
+ Coverage 93.39% 93.41% +0.02%
==========================================
Files 92 92
Lines 11214 11229 +15
==========================================
+ Hits 10473 10490 +17
+ Misses 741 739 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Pierre-Sassoulas
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.
Make sense to me (both the change and waiting for 4.1), let's hear what Jacob think.
jacobtylerwalls
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.
LGTM
This partially fixes pylint-dev/pylint#8425. The additional changes should likely be done in pylint itself.
This does impact the test output for pylint a bit as errors on decorators no longer belong to the function / class now. Not sure that can be avoided though. In any case, I think it's better to do this in
4.1.0and not in a bugfix release.