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

Feature: distinct Composition and Aggregation arrows #7836

Merged
merged 32 commits into from
Nov 27, 2022

Conversation

qequ
Copy link
Contributor

@qequ qequ commented Nov 23, 2022

Type of Changes

Type
✨ New feature

Description

Update pyreverse to differentiate between compositions and aggregations in classes. Only covers the basic cases of composition and aggregation(@DudeNr33 ).

e.g. having a code like this

""" docstring for file clientmodule.py """
from data.suppliermodule_test import Interface, DoNothing, DoNothing2

class Ancestor:
    """ Ancestor method """
    __implements__ = (Interface,)
    cls_member = DoNothing()

    def __init__(self, value):
        local_variable = 0
        self.attr = 'this method shouldn\'t have a docstring'
        self.__value = value

    def get_value(self):
        """ nice docstring ;-) """
        return self.__value

    def set_value(self, value):
        self.__value = value
        return 'this method shouldn\'t have a docstring'

class Specialization(Ancestor):
    TYPE = 'final class'
    top = 'class'

    def __init__(self, value, _id, relation2: DoNothing2):
        Ancestor.__init__(self, value)
        self._id = _id
        self.relation = DoNothing()
        self.relation2 = relation2

    @classmethod
    def from_value(cls, value: int):
        return cls(value, 0, DoNothing2())

    @staticmethod
    def transform_value(value: int) -> int:
        return value * 2

    def increment_value(self) -> None:
        self.set_value(self.get_value() + 1)

without the feature
classes_test

with the feature
classes_test

Refs #6543

CC: @dgutson

Update logic of nodes to check what kind of relationship does nodes have (association, aggregation)

Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
…links

Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Update logic of nodes to check what kind of relationship does nodes have (association, aggregation)

Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
…links

Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
@qequ qequ requested a review from DudeNr33 as a code owner November 23, 2022 21:00
@coveralls
Copy link

coveralls commented Nov 23, 2022

Pull Request Test Coverage Report for Build 3550691460

  • 48 of 50 (96.0%) changed or added relevant lines in 4 files are covered.
  • 13 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.005%) to 95.432%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pylint/pyreverse/inspector.py 30 32 93.75%
Files with Coverage Reduction New Missed Lines %
pylint/checkers/classes/class_checker.py 1 94.69%
pylint/checkers/refactoring/refactoring_checker.py 12 98.11%
Totals Coverage Status
Change from base Build 3534208420: 0.005%
Covered Lines: 17591
Relevant Lines: 18433

💛 - Coveralls

@Pierre-Sassoulas Pierre-Sassoulas added pyreverse Related to pyreverse component Needs review 🔍 Needs to be reviewed by one or multiple more persons labels Nov 23, 2022
@github-actions

This comment has been minimized.

qequ and others added 3 commits November 24, 2022 13:05
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
@github-actions

This comment has been minimized.

Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
DudeNr33
DudeNr33 previously approved these changes Nov 24, 2022
Copy link
Collaborator

@DudeNr33 DudeNr33 left a comment

Choose a reason for hiding this comment

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

Thank you very much for this contribution. This was not a trivial task, and I like the solution as it can be extended later on. 👍

@Pierre-Sassoulas Milestone 2.16.0?

doc/whatsnew/fragments/6543.feature Outdated Show resolved Hide resolved
pylint/pyreverse/inspector.py Outdated Show resolved Hide resolved
Co-authored-by: Andreas Finkler <3929834+DudeNr33@users.noreply.github.com>
Signed-off-by: Alvaro Frias Garay <alvaro.frias@eclypsium.com>
@qequ qequ requested a review from DudeNr33 November 24, 2022 19:36
@qequ qequ dismissed stale reviews from Pierre-Sassoulas and DudeNr33 via 47bccad November 25, 2022 21:00
@qequ qequ requested review from DudeNr33 and Pierre-Sassoulas and removed request for DanielNoord, DudeNr33 and Pierre-Sassoulas November 25, 2022 21:01
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component and removed Needs review 🔍 Needs to be reviewed by one or multiple more persons labels Nov 25, 2022
@qequ qequ requested review from DanielNoord and removed request for Pierre-Sassoulas November 25, 2022 21:03
@qequ qequ requested review from DudeNr33 and removed request for DanielNoord November 25, 2022 21:03
@github-actions
Copy link
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit be24c58

@DudeNr33
Copy link
Collaborator

@DanielNoord anything left to change from your side?

@DudeNr33 DudeNr33 merged commit b950567 into pylint-dev:main Nov 27, 2022
@DudeNr33
Copy link
Collaborator

Thank you @qequ for this enhancement!

@dgutson
Copy link

dgutson commented Nov 27, 2022

Go @qequ go!

@khashashin
Copy link

Thank you guys, great job 🎉

@jacobtylerwalls
Copy link
Member

@DudeNr33 Gentle reminder to wipe/reduce GitHub's suggested commit message when squashing. Thanks!

@DudeNr33
Copy link
Collaborator

@DudeNr33 Gentle reminder to wipe/reduce GitHub's suggested commit message when squashing. Thanks!

Ah, I did not know that this is the preferred way. I'll try to remember.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component pyreverse Related to pyreverse component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants