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

Promote most dunder definitions on FunctionModel to ObjectModel #1519

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented Apr 18, 2022

Description

Create DunderCompletionMixin in order to let ModuleModel share the same dunder definitions that were already present on FunctionModel.

Type of Changes

Type
βœ“ πŸ”¨ Refactor

Related Issue

Refs pylint-dev/pylint#6094

@jacobtylerwalls jacobtylerwalls added Enhancement ✨ Improvement to a component pylint-tested PRs that don't cause major regressions with pylint labels Apr 18, 2022
@coveralls
Copy link

coveralls commented Apr 18, 2022

Pull Request Test Coverage Report for Build 2447909857

  • 27 of 27 (100.0%) changed or added relevant lines in 1 file are covered.
  • 229 unchanged lines in 11 files lost coverage.
  • Overall coverage increased (+0.2%) to 92.13%

Files with Coverage Reduction New Missed Lines %
astroid/nodes/scoped_nodes/scoped_nodes.py 1 92.48%
astroid/protocols.py 1 91.84%
astroid/interpreter/_import/spec.py 3 98.24%
astroid/rebuilder.py 5 96.9%
astroid/brain/brain_namedtuple_enum.py 6 93.55%
astroid/nodes/node_ng.py 21 93.17%
astroid/raw_building.py 23 91.79%
astroid/inference.py 32 94.05%
astroid/modutils.py 38 82.95%
astroid/bases.py 40 86.59%
Totals Coverage Status
Change from base Build 2367082861: 0.2%
Covered Lines: 9319
Relevant Lines: 10115

πŸ’› - Coveralls

@jacobtylerwalls jacobtylerwalls changed the title Create DunderCompletionMixin to allow further dunder methods on ModuleModel Promote dunder methods from ModuleModel to ObjectModel Jun 7, 2022
@jacobtylerwalls jacobtylerwalls changed the title Promote dunder methods from ModuleModel to ObjectModel Create DunderCompletionMixin to allow further dunder methods on ModuleModel Jun 7, 2022
@jacobtylerwalls jacobtylerwalls changed the title Create DunderCompletionMixin to allow further dunder methods on ModuleModel Create _DunderCompletionMixin to allow further dunder methods on ModuleModel Jun 7, 2022
@DanielNoord
Copy link
Collaborator

Removing this from 2.12 as I'd like to merge #1606 first, which I'm not sure will be put into 2.12.

@DanielNoord DanielNoord removed this from the 2.12.0 milestone Jul 2, 2022
@DanielNoord
Copy link
Collaborator

Okay, I applied your diff. Merging main creates some interesting failures which makes me wonder if we missed something in #1606

I'll need to investigate that.

@DanielNoord
Copy link
Collaborator

Hm, it seems that moving these things to ObjectModel has the side effect that we now start inferring some calls to those new attributes. Since attr___init__ is defined as attr___ne__ all calls to __init__ become uninferable.

@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented Jul 3, 2022

Since attr___init__ is defined as attr___ne__ all calls to __init__ become uninferable.

πŸ€•

@jacobtylerwalls jacobtylerwalls marked this pull request as draft July 3, 2022 23:48
@DanielNoord DanielNoord force-pushed the dunder-completion branch 3 times, most recently from b9b0708 to 8888eba Compare July 6, 2022 09:16
@DanielNoord
Copy link
Collaborator

This is getting closer to completion. The only issue is now that whenever we overwrite __eq__ this doesn't resolve correctly anymore.
I'm not sure if this is for all overwrites of __eq__ or only in brains, as only the numpy brain is failing.

I tried to dig a little deeper but I would like to merge the other PRs first before tackling this one again.

@jacobtylerwalls jacobtylerwalls removed Enhancement ✨ Improvement to a component pylint-tested PRs that don't cause major regressions with pylint labels Jul 6, 2022
@cdce8p cdce8p modified the milestones: 2.14.0, 2.15.0 Jan 30, 2023
@DanielNoord DanielNoord changed the title Create _DunderCompletionMixin to allow further dunder methods on ModuleModel Promote most dunder definitions on FunctionModel to ObjectModel Feb 5, 2023
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.15.0, 2.16.0 Feb 9, 2023
@jacobtylerwalls
Copy link
Member Author

Closing as I don't have immediate plans to work on this.

@DanielNoord
Copy link
Collaborator

@jacobtylerwalls Could you restore the branch? I actually wanted to complete this πŸ˜„

@DanielNoord
Copy link
Collaborator

In the spirit of fixing stale PR's:
@jacobtylerwalls Do you have any idea about a possible fix here? The issue I'm running into above has bugged me for weeks/months and I haven't found a good solution for fixing it. I think the PR makes sense but it feels like the way astroid does inference is just flawed with respect to these dunders. Could you have a look and see if you can help out?

@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0a4, 3.0.0a5 Jun 6, 2023
@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented Jun 7, 2023

Thanks for the suggestion. I think with the time on hand, my priorities for the moment are triage, performance, and #2048. So I'm unlikely to return to this.

@Pierre-Sassoulas Pierre-Sassoulas added the Needs take over πŸ›ŽοΈ Orignal implementer went away but the code could be salvaged. label Jun 7, 2023
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0a5, 3.0.0a6 Jun 13, 2023
@jacobtylerwalls jacobtylerwalls removed this from the 3.0.0a6 milestone Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs take over πŸ›ŽοΈ Orignal implementer went away but the code could be salvaged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants