Refactor depth filtering logic

[Julfried]

Type of Changes

	Type
✓	🔨 Refactoring

Description

Refactored the depth filtering logic, as discussed in #10077. Implementation should be much cleaner now :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.84%. Comparing base (05e83c4) to head (11d109b).
Report is 14 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10200   +/-   ##
=======================================
  Coverage   95.84%   95.84%           
=======================================
  Files         175      175           
  Lines       19056    19038   -18     
=======================================
- Hits        18264    18247   -17     
+ Misses        792      791    -1     

Files with missing lines	Coverage Δ
pylint/pyreverse/diadefslib.py	99.24% <100.00%> (+0.04%)	⬆️
pylint/pyreverse/main.py	91.66% <100.00%> (ø)
pylint/pyreverse/writer.py	100.00% <ø> (+0.84%)	⬆️

... and 2 files with indirect coverage changes

[Pierre-Sassoulas]

Thank you for the merge request ! LGTM, some nits:

[Julfried]

Thanks for the review, your suggestions make sense.

One thing I noticed when playing around with it. If you call pyreverse for a subpackage and apply a depth limit. For example:

pyreverse --max-depth 1 package.subpackage 

The diagram is correctly generated and starts only at subpackage. However the depth limit is still calculated with respect to the root package. I think it would be more intuitive to calculate depth relative to subpackage in this case. What do you think?

[github-actions]

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

This comment was generated for commit 369e19d

[Julfried]

However I am unsure how to achive this given that pyreverse lets you specify multiple packages to analyse

[Pierre-Sassoulas]

Yeah hard to reconcile all the arguments. Maybe it's easier to add a warning like "You asked for depth=1 but linked package.subpackage which require depth=2 to be shown as pyreverse count depth from the root, did you meant package or depth=3 ? "

[DudeNr33]

Thank you for the refactor! I agree with Pierre, I would go for a warning right now for the case you mentioned.

[Julfried]

Thanks for the review and for the suggestion regarding the warning! For properly implementing this warning I would need to implement this in main.py so that I dont have to gather the arguments passed. Maybe it makes more sense to just document this behavior instead?

[DudeNr33]

Yes, maybe adding a short notice in the help text of the option that explains how the counting works/where it starts would be the best solution for now.

[Pierre-Sassoulas]

From experience no one ever read the documentation, so I would favor a warning (unless it's really unreasonable to do it of course).

[Julfried]

I agree with Pierre that documentation is hardly read, so a warning would be much more effective in guiding users. However, to properly detect this edge case, I would need access to the original specified by the user (self.args in main.py) inside DiadefsHandler. Right now, I don’t see a straightforward way to pass this information without modifying the argument flow, but I’ll think about possible solutions. If anyone has suggestions on a clean way to achieve this, I’d be happy to discuss!

[Julfried]

Looked into the argument flow, and changing the structure actually wasn’t too bad, so I implemented depth filtering relative to the shallowest package specified. This keeps submodules grouped under their parent package.

However, this introduces inconsistent behavior:

• With one package, depth is relative to the deepest package.
• With multiple packages, depth is relative to the shallowest package.

An alternative is to always use the deepest package as the base, ensuring all explicitly listed packages are included (e.g., pyreverse --max-depth 0 pkg pkg.subpkg would always show pkg.subpkg). To me the shallowest approach feels more intuitive for grouping. However I am unsure and open to feedback if you prefer it the other way around. Let me know what you think :)