Add type annotations to pyreverse #4551

mbyrnepr2 · 2021-06-07T15:14:31Z

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

demo.py:

from typing import Dict, List, NamedTuple


class A:
    classvar1: Dict = None
    classvar2 = 'classvar2'

    def __init__(self, a: str = None, b: NamedTuple = None, c = 1):
        self.a = a
        self.b = b
        self.c = c

    def m1(self, i: str, j: int) -> List:
        pass

development branch:

master branch:

Type of Changes

	Type
✓	✨ New feature

Related Issue

coveralls · 2021-06-07T15:24:48Z

Coverage increased (+0.02%) to 92.036% when pulling 194e6d3 on mbyrnepr2:1548_pyreverse_type_annotations into 99589b0 on PyCQA:master.

Closes pylint-dev#1548

Pierre-Sassoulas

Thank you for this changes :) I made some suggestions that could maybe simplify the code let me know if there is something that I did not see :)

Pierre-Sassoulas · 2021-06-14T07:02:34Z

pylint/pyreverse/inspector.py

+            ann = getattr(node.parent, "annotation", None)
+            values = {ann} if ann else set(node.infer())


Suggested change

ann = getattr(node.parent, "annotation", None)

values = {ann} if ann else set(node.infer())

values = {getattr(node.parent, "annotation", node.infer())}

Would that work ?

Pierre-Sassoulas · 2021-06-14T07:04:13Z

pylint/pyreverse/inspector.py

@@ -229,8 +230,17 @@ def handle_assignattr_type(node, parent):

        handle instance_attrs_type
        """
+        ann = None


Suggested change

ann = None

ann = node.infer()

I think doing that would simplify that following code.

mbyrnepr2 · 2021-06-14T07:11:39Z

@Pierre-Sassoulas thank you! I'll try that later today along with the further changes required to display variables a optional in the diagram.

alexchandel · 2021-06-15T20:13:58Z

This would be fantastic! There is no Python UML generator that understands type annotations right now. This also lays the groundwork for the future generation of dependency arrows for class attributes.

@mbyrnepr2 Did the requested changes work?

mbyrnepr2 · 2021-06-15T20:24:23Z

@alexchandel the suggested changes work perfectly. Aiming to update the pr with those + Optional output this week.

Pierre-Sassoulas · 2021-06-16T04:46:24Z

@DudeNr33 I don't know if you saw that MR but someone else if working on pyreverse in parallel :) Maybe you can rebase on it as soon as it's merged to avoid handling conflicts late.

DudeNr33 · 2021-06-16T05:00:40Z

Thanks for the heads up!
Great work, I actually also had the corresponding issue bookmarked for some time now because I think this is a great improvement! Will rebase my own branch as soon as I got time.

- Indicate the attribute is optional in the dot files by inspecting the default value. - Handle the Subscript type annotations - Refactor & move the logic to obtain annotations to utils.py as re-usable functions - Add unittests for the added functions

…type_annotations

Pierre-Sassoulas

Thank you for going back to this. I think there's should be a handling for the inference fail that can happen with infer() call. I don't have an example of code where there inference would fail, maybe we can patch NodeNg so it raises one in order to be able to test ?

Pierre-Sassoulas · 2021-06-24T17:06:38Z

pylint/pyreverse/inspector.py

@@ -218,7 +218,8 @@ def visit_assignname(self, node):
                    self.visit_module(frame)

            current = frame.locals_type[node.name]
-            values = set(node.infer())
+            ann = utils.get_annotation(node)
+            values = {ann} if ann else set(node.infer())


Here there can be an inference fail, we should catch it (like line 238). Btw it look like the code is similar, maybe we can create a function for it ?

Pierre-Sassoulas · 2021-06-24T17:08:32Z

pylint/pyreverse/utils.py

+    else:
+        return ann
+
+    default, *_ = node.infer()


There can be an InferenceError raised here.

- Add a try/except to deal with a possible InferenceError when using NodeNG.infer method - Create a function in utils and so remove repeated logic in inspector.py - Add unittests to check the InferenceError logic - Adjust the types in function input

mbyrnepr2 · 2021-06-26T16:12:33Z

Thanks again @Pierre-Sassoulas I agree with all the above. I've aimed to address the points in 194e6d3. I could easily have missed something though so happy to look again if something doesn't look right.

Pierre-Sassoulas

Thank you for all the work that went into this !

mbyrnepr2 force-pushed the 1548_pyreverse_type_annotations branch from b485530 to 043c883 Compare June 7, 2021 15:17

mbyrnepr2 force-pushed the 1548_pyreverse_type_annotations branch 3 times, most recently from ead9a4c to caed9b0 Compare June 9, 2021 20:33

Add type annotations to pyreverse dot files

4b35586

Closes pylint-dev#1548

mbyrnepr2 force-pushed the 1548_pyreverse_type_annotations branch from caed9b0 to 4b35586 Compare June 10, 2021 20:05

mbyrnepr2 marked this pull request as ready for review June 10, 2021 20:14

Pierre-Sassoulas added pyreverse Related to pyreverse component Enhancement ✨ Improvement to a component labels Jun 14, 2021

Pierre-Sassoulas requested changes Jun 14, 2021

View reviewed changes

Pierre-Sassoulas mentioned this pull request Jun 17, 2021

pyreverse missing dependency injection? #4585

Closed

mbyrnepr2 and others added 3 commits June 20, 2021 18:12

Merge branch 'master' of github.com:PyCQA/pylint into 1548_pyreverse_…

4dc5dc5

…type_annotations

Merge branch 'master' into 1548_pyreverse_type_annotations

1d3d13b

mbyrnepr2 requested a review from Pierre-Sassoulas June 24, 2021 14:35

Pierre-Sassoulas requested changes Jun 24, 2021

View reviewed changes

Pierre-Sassoulas approved these changes Jun 26, 2021

View reviewed changes

Pierre-Sassoulas added this to the 2.9.0 milestone Jun 26, 2021

Pierre-Sassoulas merged commit 1e55ae6 into pylint-dev:master Jun 26, 2021

Pierre-Sassoulas mentioned this pull request Jun 26, 2021

Pyreverse autocolor #4521

Closed

3 tasks

DudeNr33 mentioned this pull request Aug 14, 2021

pyreverse: add PlantUML output #4846

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add type annotations to pyreverse #4551

Add type annotations to pyreverse #4551

mbyrnepr2 commented Jun 7, 2021 •

edited

coveralls commented Jun 7, 2021 •

edited

Pierre-Sassoulas left a comment

Pierre-Sassoulas Jun 14, 2021

Pierre-Sassoulas Jun 14, 2021

mbyrnepr2 commented Jun 14, 2021

alexchandel commented Jun 15, 2021

mbyrnepr2 commented Jun 15, 2021

Pierre-Sassoulas commented Jun 16, 2021

DudeNr33 commented Jun 16, 2021

Pierre-Sassoulas left a comment

Pierre-Sassoulas Jun 24, 2021

Pierre-Sassoulas Jun 24, 2021

mbyrnepr2 commented Jun 26, 2021

Pierre-Sassoulas left a comment

		ann = getattr(node.parent, "annotation", None)
		values = {ann} if ann else set(node.infer())

	ann = getattr(node.parent, "annotation", None)
	values = {ann} if ann else set(node.infer())
	values = {getattr(node.parent, "annotation", node.infer())}

Add type annotations to pyreverse #4551

Add type annotations to pyreverse #4551

Conversation

mbyrnepr2 commented Jun 7, 2021 • edited

Steps

Description

Type of Changes

Related Issue

coveralls commented Jun 7, 2021 • edited

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas Jun 14, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas Jun 14, 2021

Choose a reason for hiding this comment

mbyrnepr2 commented Jun 14, 2021

alexchandel commented Jun 15, 2021

mbyrnepr2 commented Jun 15, 2021

Pierre-Sassoulas commented Jun 16, 2021

DudeNr33 commented Jun 16, 2021

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas Jun 24, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas Jun 24, 2021

Choose a reason for hiding this comment

mbyrnepr2 commented Jun 26, 2021

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

mbyrnepr2 commented Jun 7, 2021 •

edited

coveralls commented Jun 7, 2021 •

edited