Pyreverse: Aggregation/compostion doesn't work across modules.

[bMorgan01]

Bug description

I have a project with this structure:

pyreverse_test
     __init__.py
     a.py
     b.py

a.py contains:

class A:
    def __init__(self) -> None:
        self.var = 2

b.py contains:

from a import A

class B:
    def __init__(self) -> None:
        self.a_obj: A = A()

Running pyreverse yields this result:

Configuration

No response

Command used

Working directory: pyreverse_test

pyreverse . -f ALL -o png

Pylint output

Format png is not supported natively. Pyreverse will try to generate it using Graphviz...
Analysed 3 modules with a total of 1 imports

Expected behavior

I expect the result to match this, un-separated, code.

class A:
    def __init__(self) -> None:
        self.var = 2

class B:
    def __init__(self) -> None:
        self.a_obj: A = A()

Which is this:

Pylint version

pylint 3.0.2
astroid 3.0.1
Python 3.10.11 (tags/v3.10.11:7d4cc5a, Apr  5 2023, 00:38:17) [MSC v.1929 64 bit (AMD64)]

OS / Environment

Windows 10 / powershell

Additional dependencies

No response

[nickdrozd]

This works in some cases but not others. I have no idea why.

[Julfried]

This issue is interesting - I found that PyReverse correctly detects the aggregation relationship with relative imports (using dot notation: from .a import A), but may fail with non-relative imports (from a import A). I think this is expected and I am not sure if this is really a bug. What do you think?

[Pierre-Sassoulas]

In my opinion we should "normalize" the import and treat both way of importing the same in pyreverse if the final result is equivalent in python. Let's hear other opinions before taking a decision.

[Julfried]

Agree, I think we should treat the imports "normalized"

[Julfried]

Actually thinking about it more , we should not normalize the paths. Looking at the provided example this is actually invalid python code as far as I understand the import system. When running this test script:

from pyreverse_test import b

test = b.B()

I get an import error:

Traceback (most recent call last):
  File "c:\WORKSPACE\pylint\test.py", line 1, in <module>
    from pyreverse_test import b
  File "c:\WORKSPACE\pylint\pyreverse_test\b.py", line 1, in <module>
    from a import A
ModuleNotFoundError: No module named 'a'

And this actually makes sense because you are trying to absolut import the class from within the package context. Using relative imports (from .a import A) makes the script run without any errors and pyreverse also produces the expected output

Output from pyreverse .\pyreverse_test\ -o mmd -f ALL:

classDiagram
  class A {
    var : int
    \_\_init\_\_() None
  }
  class B {
    a_obj
    \_\_init\_\_() None
  }
  A --* B : a_obj

In my opinion pyreverse should focus on clear error messages when imports can't be resolved, instead of silently normalizing invalid imports. @Pierre-Sassoulas What do you think?

[Pierre-Sassoulas]

Yes, 'from pyreverse_test.a import A' should work the same as a relative import 'from .a import A', but wrong imports shouldn't work.

[Julfried]

Can confirm. Both from .a import A and from pyreverse_test.a import A produce the correct output.

The invalid import from a import A produces this output as expected:

classDiagram
  class A {
    var : int
    \_\_init\_\_() None
  }
  class B {
    a_obj
    \_\_init\_\_() None
  }

[bMorgan01]

The invalid import from a import A produces this output as expected:

Why is from a import A invalid?

Edit: saw your error traceback from earlier.
I dont know if this is a "works on my machine" situation, but that's how I've always done local project imports

I think if you put the main function in b.py instead, the import would work as expected