-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Now TypeInfo.get_method
also returns Decorator
nodes
#11150
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
Conversation
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.
Thanks for the PR! Left a few comments and a question.
This commit just addresses the code review: 28d950d Now, I am going to change how |
This comment has been minimized.
This comment has been minimized.
I am running self-check with the only change:
Looks doable! 👍 |
This is now pretty big:
|
else: | ||
result = getattr_type | ||
|
||
# Call the attribute hook before returning. |
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.
Now plugin hooks are also triggered for non callable
types as well.
__getattr__
worksTypeInfo.get_method
also returns Decorator
nodes
Ok, looks like @JukkaL any tips on that? I haven't used |
if isinstance(node, (FuncBase, Decorator)):
return node But, it accepted this code: if isinstance(node, FuncBase):
return node
elif isinstance(node, Decorator):
return node Seems like a bug in |
I think I found a reproducer for the mypyc issue: mypyc/mypyc#892 |
@JukkaL awesome! Thanks! Is there anything else left to do here? |
Rebased on |
This comment has been minimized.
This comment has been minimized.
@JukkaL any chance to get this merged? 🙂 |
@JukkaL rebased once again 🙂 I will also friendly ping @hauntsaninja as well. |
This comment has been minimized.
This comment has been minimized.
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.
Using MemberContext
in analyze_descriptor_access
is a good refactoring!
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.
Thanks for the PR, this looks solid! I like how this fixes many related issues.
|
||
function = function_type(dunder_set, self.named_type('builtins.function')) | ||
bound_method = bind_self(function, attribute_type) | ||
if isinstance(dunder_set, Decorator): |
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.
Code quite similar to this if/else gets repeated several times. I wonder if it would be possible to abstract it into a helper function, for example? If this sounds like a feasible idea, it could be a nice follow-up PR.
@JukkaL thank you! I will think about a helper function 🙂 |
Support decorators properly in additional contexts. Closes python#10409
* Follow up to python#11150, refactors `if`s into a util function
Closes #10409