Skip to content

Handle has-a relationships for type-hinted arguments in class diagrams #4745

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jul 30, 2021
Merged

Handle has-a relationships for type-hinted arguments in class diagrams #4745

merged 7 commits into from
Jul 30, 2021

Conversation

mbyrnepr2
Copy link
Member

@mbyrnepr2 mbyrnepr2 commented Jul 25, 2021 •
edited
Loading

Steps

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Description

We get a KeyError on this line https://github.com/PyCQA/pylint/blob/main/pylint/pyreverse/diagrams.py#L194 because the value is not of type ClassDef and hence cannot be found in the self._nodes whose keys are all of that type.

Type of Changes

Type
🐛 Bug fix
✓ ✨ New feature
🔨 Refactoring
📜 Docs

Related Issue

Closes #4744

@coveralls
Copy link

coveralls commented Jul 25, 2021 •
edited
Loading

Coverage Status

Coverage increased (+0.007%) to 92.259% when pulling c3e96af on mbyrnepr2:pyreverse_type_hints_and_has_relationships into 5e5f48d on PyCQA:main.

@mbyrnepr2 mbyrnepr2 force-pushed the pyreverse_type_hints_and_has_relationships branch 2 times, most recently from 5b0f980 to 3448d4e Compare July 25, 2021 16:10
@mbyrnepr2 mbyrnepr2 force-pushed the pyreverse_type_hints_and_has_relationships branch from 3448d4e to 846415d Compare July 25, 2021 16:22
@mbyrnepr2 mbyrnepr2 marked this pull request as ready for review July 25, 2021 16:24
Pierre-Sassoulas
Pierre-Sassoulas requested changes Jul 25, 2021
View reviewed changes
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.

Looks nice, could you add one or two automated tests for this, please ?

@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Jul 25, 2021
@mbyrnepr2
Copy link
Member Author

Looks nice, could you add one or two automated tests for this, please ?

Agree. Will address asap.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.10.0 milestone Jul 26, 2021
@DudeNr33 DudeNr33 mentioned this pull request Jul 28, 2021
Merged
1 task
Pierre-Sassoulas and others added 5 commits July 29, 2021 20:50
@Pierre-Sassoulas
Merge branch 'main' into pyreverse_type_hints_and_has_relationships
535f494
@mbyrnepr2
Merge branch 'main' of github.com:PyCQA/pylint into pyreverse_type_hi…
468a312 
…nts_and_has_relationships
@mbyrnepr2
Pyreverse - Show class has-a relationships inferred from type-hints
7744a27 
Add tests
- Unittest to check returned type is a ClassDef
- Update pyreverse test/data to include the scenario of a relation
inferred from a type-hint

Closes pylint-dev#4744
@mbyrnepr2
Merge branch 'pyreverse_type_hints_and_has_relationships' of github.c…
7da390d 
…om:mbyrnepr2/pylint into pyreverse_type_hints_and_has_relationships
@mbyrnepr2
Pyreverse - Show class has-a relationships inferred from type-hints
c3e96af 
Rm duplicate changelog entry
@mbyrnepr2
Copy link
Member Author

Sorry for the delay @Pierre-Sassoulas @DudeNr33. Added some testing. I will have no access to a laptop for the next couple of days but can respond occasionally if there are any issues.

@DudeNr33
Copy link
Collaborator

No worries. My PR is nothing urgent, it can wait a bit longer.

Pierre-Sassoulas
Pierre-Sassoulas approved these changes Jul 30, 2021
View reviewed changes
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.

Thank you for adding the test this looks great !

@Pierre-Sassoulas Pierre-Sassoulas merged commit b71be8a into pylint-dev:main Jul 30, 2021
cdce8p added a commit to cdce8p/pylint that referenced this pull request Aug 1, 2021
@cdce8p
PR 4783 - nested attributes
b0164a6 
Squashed commit of the following:

commit 49c4bba
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Aug 1 19:56:51 2021 +0800

    Fix crash for `unused-private-member` when there are nested attributes

commit 2ad8247
Merge: 8ceb26d 1d09bc8
Author: yushao2 <[email protected]>
Date:   Sun Aug 1 20:13:05 2021 +0800

    Merge pull request pylint-dev#4709 from yushao2/fix-unused-private-member-4673

    [unused-private-member] add logic for checking nested functions

commit 1d09bc8
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Aug 1 20:03:42 2021 +0800

    update pr based on review

commit a4198cd
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Aug 1 19:21:36 2021 +0800

    Update pr based on review

commit c8b2cbb
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Sun Jul 25 05:20:42 2021 +0000

    [pre-commit.ci] auto fixes from pre-commit.com hooks

    for more information, see https://pre-commit.ci

commit 4ffea0b
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Jul 25 13:12:29 2021 +0800

    Update pr based on review

commit e4d6243
Author: Marc Mueller <[email protected]>
Date:   Sun Jul 18 17:23:31 2021 +0200

    Remove empty line

commit cce5833
Author: Yu Shao, Pang <[email protected]>
Date:   Sun Jul 18 22:40:54 2021 +0800

    update PR based on comments

commit 712dc2b
Author: Yu Shao, Pang <[email protected]>
Date:   Wed Jul 14 16:42:25 2021 +0800

    [unused-private-member] add logic for checking nested functions

    also, improve error message for nested functions

commit 8ceb26d
Author: Michal Vasilek <[email protected]>
Date:   Sun Aug 1 08:14:58 2021 +0200

    Fix IsADirectoryError in tests/lint/unittest_lint (pylint-dev#4781)

    pylintd is a directory, so os.remove throws IsADirectoryError

commit a31e6bc
Author: Pierre Sassoulas <[email protected]>
Date:   Sat Jul 31 11:21:46 2021 +0200

    Add documentation for useless-suppression

    Closes pylint-dev#4757

commit b71be8a
Author: Mark Byrne <[email protected]>
Date:   Fri Jul 30 20:21:02 2021 +0200

    Handle has-a relationships for type-hinted arguments in class diagrams (pylint-dev#4745)

    * Pyreverse - Show class has-a relationships inferred from type-hints

    Closes pylint-dev#4744

    Co-authored-by: Pierre Sassoulas <[email protected]>

commit 5e5f48d
Author: Daniël van Noord <[email protected]>
Date:   Thu Jul 29 23:44:30 2021 +0200

    Add ``forgotten-debug-statement`` checker (pylint-dev#4771)

    * Add ``no-breakpoint`` checker this adds a checker for calls to ``breakpoint()``, ``pdb.set_trace()``, or ``sys.breakpointhook()``.

    Closes pylint-dev#3692

    Co-authored-by: Marc Mueller <[email protected]>
    Co-authored-by: Pierre Sassoulas <[email protected]>
@mbyrnepr2 mbyrnepr2 deleted the pyreverse_type_hints_and_has_relationships branch May 28, 2022 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pierre-Sassoulas Pierre-Sassoulas Pierre-Sassoulas approved these changes

Assignees
No one assigned
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.

pyreverse: has-a relationships for type-hinted arguments
4 participants
@mbyrnepr2 @coveralls @DudeNr33 @Pierre-Sassoulas