Skip to content
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 __init__ to the ObjectModel and return BoundMethods #1687

Merged
merged 5 commits into from
Jul 6, 2022

Conversation

DanielNoord
Copy link
Collaborator

@DanielNoord DanielNoord commented Jul 6, 2022

Steps

  • Write a good description on what the PR does.

Description

Type of Changes

Type
βœ“ πŸ”¨ Refactoring

Related Issue

Blocked by #1686
Precedes #1519

@jacobtylerwalls Please only consider the last commit. This is the next step in "completing" our ObjectModel. Since all objects have an __init__ that normally returns None we can define attr___init__ on ObjectModel.

@DanielNoord DanielNoord added the Enhancement ✨ Improvement to a component label Jul 6, 2022
@DanielNoord DanielNoord added this to the 2.12.0 milestone Jul 6, 2022
@coveralls
Copy link

coveralls commented Jul 6, 2022

Pull Request Test Coverage Report for Build 2622711369

  • 21 of 21 (100.0%) changed or added relevant lines in 1 file are covered.
  • 4 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 92.127%

Files with Coverage Reduction New Missed Lines %
astroid/interpreter/objectmodel.py 1 92.62%
astroid/nodes/node_ng.py 1 92.81%
astroid/bases.py 2 87.35%
Totals Coverage Status
Change from base Build 2622607134: -0.02%
Covered Lines: 9443
Relevant Lines: 10250

πŸ’› - Coveralls

@DanielNoord DanielNoord changed the title Add __init__ to the ObjectModel Add __init__ to the ObjectModel and return BoundMethods Jul 6, 2022
@DanielNoord DanielNoord added the pylint-tested PRs that don't cause major regressions with pylint label Jul 6, 2022
@DanielNoord
Copy link
Collaborator Author

After testing on pylint I found that there were still issues with the previous implementation.

I think now I (finally) fixed those.

I was mistaken to think that the other attr___ methods are correct. Rather than returning the value directly these methods should return the BoundMethod of the method that is going to be called. The functools brain uses a somewhat similar trick:
https://github.com/PyCQA/astroid/blob/58750a6f982c20fd7224ac396c95ee8adbbfa34f/astroid/brain/brain_functools.py#L52-L55

We will infer the result of these BoundMethods succesfully by calling infer_call_result on them, but for some cases we need the BoundMethod itself. For example for the following code A().__init__ #@. That shouldn't be a nodes.Const, but the BoundMethod that will return nodes.Const.

Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Looks good!

astroid/interpreter/objectmodel.py Outdated Show resolved Hide resolved
astroid/interpreter/objectmodel.py Outdated Show resolved Hide resolved
astroid/interpreter/objectmodel.py Outdated Show resolved Hide resolved
DanielNoord and others added 2 commits July 6, 2022 14:08
Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
@jacobtylerwalls
Copy link
Member

This feels like a new feature to some degree. What do you think about a short changelog?

Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

πŸŽ‰

@DanielNoord DanielNoord merged commit 9224558 into pylint-dev:main Jul 6, 2022
@DanielNoord DanielNoord deleted the modmodel branch July 6, 2022 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component pylint-tested PRs that don't cause major regressions with pylint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants