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
Add Arguments
typing to Lambda
& FunctionDef
#1174
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.
Looks good overall. I'm going to merge that first. You should be able to remove the pylint: disable
calls from #1170 then.
|
||
:type: Arguments or list | ||
""" | ||
self.args: Arguments |
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.
At some point we should probably enforce that postinit
is actually called. Otherwise self.args
might not be set when accessed. Task for another time. (It's somewhere on my long list.)
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.
This is already note the case.. Of the two places creating a FunctioDef
only one does this. Oh well... π
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.
Which one do you mean? raw_building
, objectmodel
, and rebuilder
are fine. Am I missing something? I didn't notice any errors while running the tests either.
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 postinit
is called on Arguments
, but the FunctionDef.postinit
method is never called.
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.
At least func.args
is still assigned. So it won't break (just yet).
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.
Something to fix in the future I guess. Seems like the Lambda
and FunctionDef
class could use a refactor anyway.
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.
Someday... Too many other things open at the moment πͺ
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.
Someday... Too many other things open at the moment
Yeah let's finish up 2.11 first and the typing MR first. The code is getting better little by little and that's great, but let's merge what we did already. There are a lot of changes in those, and conflicts could become a problem.
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.
I expect to have pylint-dev/pylint#4980 ready for review in 5-10 minutes!
Steps
Description
This should fix some of the problems we were having with
args.???
in #1170.See this comment #1170 (comment)
This should make it possible to remove the disable's added in that PR.
/CC @cdce8p
Type of Changes
Related Issue