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

Pyreverse: Duplicated class variables #8046

Open
Franco0700 opened this issue Jan 10, 2023 · 4 comments
Open

Pyreverse: Duplicated class variables #8046

Franco0700 opened this issue Jan 10, 2023 · 4 comments
Labels
Enhancement ✨ Improvement to a component pyreverse Related to pyreverse component

Comments

@Franco0700
Copy link

Franco0700 commented Jan 10, 2023

Bug description

from typing import Optional

class ExampleClass():
    example1 : Optional[int] = None
    example2 : Optional[int] = None
    def __init__(self):
        self.example1 = 1
        self.example2 = 2

Configuration

No response

Command used

pyreverse -o plantuml toy_code.py

Pylint output

@startuml classes_toy_code
set namespaceSeparator none
class "ExampleClass" as parser.toy_code.ExampleClass {
  example1 : Optional[int]
  example1 : int
  example2 : Optional[int]
  example2 : int
}
@enduml

Expected behavior

@startuml classes_toy_code
set namespaceSeparator none
class "ExampleClass" as parser.toy_code.ExampleClass {
  example1 : Optional[int]
  example2 : Optional[int]
}
@enduml

Pylint version

pylint 2.16.0-dev
astroid 2.12.13
Python 3.10.8 (main, Nov  1 2022, 14:18:21) [GCC 12.2.0]

OS / Environment

5.15.85-1-MANJARO

Additional dependencies

No response

@Franco0700 Franco0700 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 10, 2023
@Pierre-Sassoulas Pierre-Sassoulas added the pyreverse Related to pyreverse component label Jan 10, 2023
@nickdrozd
Copy link
Contributor

A slightly simpler reproducing example:

class ExampleClass():
    example1: int
    example2: int

    def __init__(self):
        self.example1 = 1
        self.example2 = 2

Output:

@startuml classes
set namespaceSeparator none
class "ExampleClass" as asdf.ExampleClass {
  example1 : int
  example1 : int
  example2 : int
  example2 : int
}
@enduml

It seems like it doesn't understand the difference between declaration and assignment.

@nickdrozd nickdrozd added Bug 🪲 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 4, 2023
@DudeNr33
Copy link
Collaborator

DudeNr33 commented Feb 5, 2023

There is a subtle difference between the original example and the new one.

In the original example, you have both class attributes example1 and example2 with value None, and instance attributes example1 and example2 with integer values.
Those exist independently from each other, and you can access both values for example1 from a class instance (through a classmethod or via instance.__class__.example1).

So the original example comes down to differentiating class attributes from instance attributes.
Looking at static features in UML and the example for SearchService.createEngine here the correct way would be to show the class attribute with an underline and the instance attribute without.

For your (@nickdrozd) example there should only be an instance attribute.

@dgutson
Copy link

dgutson commented Feb 5, 2023

Let's track both bugs on two separate issues? Or the root cause is the same?

@DudeNr33
Copy link
Collaborator

DudeNr33 commented Feb 5, 2023

I created a new issue for Nick's example:
#8189

I also changed the label for this issue from Bug to Enhancement, as it is more fitting I think.

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

No branches or pull requests

5 participants