Add --max-depth option to control diagram complexity

[Julfried]

Type of Changes

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

Description

This PR adds a new --max-depth option to pyreverse that allows basic controlling of the visualization depth of packages and classes in the generated diagrams. The depth is simply calculated by counting dots in qualified names, where depth 0 represents top-level packages/classes.

For packages, the full path depth is considered. For classes, the depth is calculated based on their containing module to ensure meaningful class diagrams while respecting the depth limit.

Closes #9233

[codecov]

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.80%. Comparing base (054f233) to head (85722e6).

Files with missing lines	Patch %	Lines
pylint/pyreverse/writer.py	96.29%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10077   +/-   ##
=======================================
  Coverage   95.80%   95.80%           
=======================================
  Files         174      174           
  Lines       18976    19004   +28     
=======================================
+ Hits        18180    18207   +27     
- Misses        796      797    +1     

Files with missing lines	Coverage Δ
pylint/pyreverse/main.py	91.66% <ø> (ø)
pylint/testutils/pyreverse.py	98.11% <100.00%> (+0.03%)	⬆️
pylint/pyreverse/writer.py	99.15% <96.29%> (-0.85%)	⬇️

[Julfried]

Quick update: I've completed the implementation and added tests for the --max-depth option. I believe the feature is working as intended and is ready for review. I’m currently fighting some CI issues, but they seem unrelated to my changes. Any help or advice would be much appreciated!

[Pierre-Sassoulas]

LGTM ! Don't worry about the 3.13 CI fail, this is on main too we can rebase once it's fixed. I'll also wait for a review from Andrea before merging.

[Julfried]

Thanks! Appreciate your review and the clarification about the CI issue. I`ll keep an eye on any updates and am happy to rebase if needed.

[Pierre-Sassoulas]

Thank you for rebasing, the mypy error is coming from main the other errors seems genuine. (Spelling is checked only in ci not in local pre-commit)

[Julfried]

Thanks for pointing out the mypy error, I`ll keep an eye on it if it gets fixed in main. How about disabling spelling check for these test cases because "-" is not allowed in Python naming anyway?

[Julfried]

It seems that the spelling checks still fail, because it checks for words and ignores the quotes. How about adding these testnames to .pylintrc spelling dictionary?

[SPELLING]
# List of comma separated words that should not be checked.
spelling-ignore-words=subpkg,MyClass

[Julfried]

When working on test coverage I noticed tests/data/init.py is empty, which impacts pyreverse's ability to detect complete package relationships. This effects validation of the --max-depth functionality since module relationships aren't fully established and the resulting diagrams do not represent the whole structure.

@Pierre-Sassoulas do you maybe remember if this was an intentional design choice, or did this just grow over time?

Note: Addressing this would require modifying all affected test files to properly reflect package relationships. Given the scope, I think handling such changes in a separate PR would be better, if we decide to do this!

[Pierre-Sassoulas]

I'm not sure. Pylint had a lot of issues with namespace package and missing __init__.py in the past, is it the issue ? If you meant 'empty' and not 'missing' I'm not sure that an empty init.py should influence the result ?

[Julfried]

Thanks for checking! You're right that it's about an empty (not missing) init.py. The difference is how many relationships pyreverse detects:

• Empty init.py: Shows only direct module dependencies empty init file
• init.py with imports: Shows both module dependencies and package-level relationships imports in init file

I think this effects depth filtering coverage since we're not testing package-level relationship filtering. Although I want to add, that I am not a testing expert.

Here is what I added to init.py:

from .clientmodule_test import Ancestor, Specialization
from .suppliermodule_test import Interface, DoNothing, DoNothing2
from .nullable_pattern import NullablePatterns
from .property_pattern import PropertyPatterns

[github-actions]

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

This comment was generated for commit 85722e6

[Julfried]

Thanks for your earlier guidance, Pierre! I hope this is PR is not getting to large.

I now rebased to fix the mypy error and addressed the spelling issues. Also, I made some mistakes when setting up the tests, which resulted in pyreverse generating wrong diagrams. Now I use a proper test setup with fixtures and test multiple depths, to improve test coverage. The issue with pyreverse not properly detecting all relationships when init.py is empty still remains. I believe this is why coverage for package-level relationships is still incomplete.

In my understanding, an empty init.py file limits the ability to fully establish package-level relationships, as it doesn’t define imports or explicitly link modules in the package. This might explain why pyreverse struggles with package-level filtering in such cases. However, I’m not entirely sure if this is the root cause or if there’s another factor at play.

I’m happy to explore this further and improve the tests in a follow-up PR if needed. Let me know what you think🙂

[Pierre-Sassoulas]

Sorry for the delay. Changes seems sensible, I have a possible refactor to suggest. (I'd like a review from Andras if possible though, no pressure @DudeNr33 ;) )

[Pierre-Sassoulas]

Suggested change

	for rel in diagram.get_relationships("specialization"):
	for rel in diagram.get_relationships("specialization", depth=self.max_depth):

Wonder if it's possible of filtering on depth directly in get_relationships instead. Seems like it could reduce duplication of .should_show_node checks when looping on it

[Julfried]

Thanks for the suggestion! I would agree, that handling everything regarding filtering in one place would benefit maintainability. However I think we still need node filtering when emitting nodes, to determine which ones to show in the final diagram. Not sure about splitting this up into 2 files either. What do you think?

[Julfried]

Actually, I feel like we could simplify this, by introducing get_nodes() alongside get_relationships(). Then the writer would look something like this.

def write_packages(self, diagram: PackageDiagram) -> None:
    for module in diagram.get_nodes(depth=self.max_depth):
        # Emit node logic
    for rel in diagram.get_relationships(depth=self.max_depth):
        # Emit edge logic

That way all the filtering logic would be handled consistently inside the diagram classes and the printer would only need to focus on rendering the final components. What do you think?

[Pierre-Sassoulas]

Looks great !

[Julfried]

Since this is a big change, I am not sure how to approach this. Are you ok with changing it in this PR?

[Pierre-Sassoulas]

Right, let's open another PR for the refactor required to add the depth argument.

[Julfried]

No worries, Currently I am also very packed. Will try to take a look at it later this week :)