Skip to content

Promoted getattr() from FunctionDef to Lambda #1472

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

Merged
merged 7 commits into from
Mar 12, 2022

Conversation

jacobtylerwalls
Copy link
Member

Steps

  • For new features or bug fixes, add a ChangeLog entry describing what your PR does.
  • Write a good description on what the PR does.

Description

Type of Changes

Type
✨ New feature

Related Issue

Lambdas can have special attributes, such as __code__, or even arbitrary attributes. Attempting to use getattr on the scoped_nodes.Lambda instance, though, raised AttributeError.

Now, it won't: the PR promotes getattr() from FunctionDef to its parent, Lambda.

Discovered by pylint's primer tests during an attempt to remove an except: AttributeError from pylint's TypeChecker, discussed here.

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just adding some typing while we're at it 😄

Nice that that test suddenly works 👍

Edit: Btw, not sure if you have done this, but we might want to do a quick search in the open issues on pylint and astroid to see if that inference change for lambda's does not fix anything else .

jacobtylerwalls and others added 4 commits March 12, 2022 12:11
Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider this a bug fix and test @Pierre-Sassoulas's new release process 😄 Considering the fact that this is breaking in the pylint primer and we're handling the bug (and we're inferring more than we previously did), there is an argument to be made to do so. I'll leave that to the others though.

We would only need to change the position of the changelog entry.

Thanks @jacobtylerwalls 👍

@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented Mar 12, 2022

Makes sense. But to be clear, it's not urgent -- nothing is blocking pylint -- we were just exploring whether we could remove an except case in pylint while working on pylint-dev/pylint#5544. However, at this point, any changes would be better off in a new PR with pylint regression tests.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.11.1 milestone Mar 12, 2022
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's put this in 2.11.1 changelog :)

@Pierre-Sassoulas Pierre-Sassoulas merged commit bedb3ed into pylint-dev:main Mar 12, 2022
@jacobtylerwalls jacobtylerwalls deleted the lambda-getattr branch March 12, 2022 18:41
Pierre-Sassoulas pushed a commit that referenced this pull request Mar 22, 2022
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants