-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add --max-depth option to control diagram complexity #10077
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
Changes from 19 commits
af5fc28
0c159a1
b421afa
da49687
f35d822
39e0eac
f2a9299
1f31feb
2006bb7
1c107ee
4be3aa7
6defb5f
88f3a9d
35c062c
907479b
012d136
24fdc46
59a720e
85722e6
94a3898
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Add --max-depth option to pyreverse to control diagram complexity. A depth of 0 shows only top-level packages, 1 shows one level of subpackages, etc. | ||
This helps manage visualization of large codebases by limiting the depth of displayed packages and classes. | ||
|
||
Refs #10077 |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -35,6 +35,7 @@ | |||||
self.printer: Printer # defined in set_printer | ||||||
self.file_name = "" # defined in set_printer | ||||||
self.depth = self.config.max_color_depth | ||||||
self.max_depth = self.config.max_depth | ||||||
# default colors are an adaption of the seaborn colorblind palette | ||||||
self.available_colors = itertools.cycle(self.config.color_palette) | ||||||
self.used_colors: dict[str, str] = {} | ||||||
|
@@ -53,6 +54,38 @@ | |||||
self.write_classes(diagram) | ||||||
self.save() | ||||||
|
||||||
def should_show_node(self, qualified_name: str, is_class: bool = False) -> bool: | ||||||
"""Determine if a node should be shown based on depth settings. | ||||||
|
||||||
Depth is calculated by counting dots in the qualified name: | ||||||
- depth 0: top-level packages (no dots) | ||||||
- depth 1: first level sub-packages (one dot) | ||||||
- depth 2: second level sub-packages (two dots) | ||||||
|
||||||
For classes, depth is measured from their containing module, excluding | ||||||
the class name itself from the depth calculation. | ||||||
""" | ||||||
# If no depth limit is set ==> show all nodes | ||||||
if self.max_depth is None: | ||||||
return True | ||||||
|
||||||
# For classes, we want to measure depth from their containing module | ||||||
if is_class: | ||||||
# Get the module part (everything before the last dot) | ||||||
last_dot = qualified_name.rfind(".") | ||||||
if last_dot == -1: | ||||||
module_path = "" | ||||||
else: | ||||||
module_path = qualified_name[:last_dot] | ||||||
|
||||||
# Count module depth | ||||||
module_depth = module_path.count(".") | ||||||
return bool(module_depth <= self.max_depth) | ||||||
|
||||||
# For packages/modules, count full depth | ||||||
node_depth = qualified_name.count(".") | ||||||
return bool(node_depth <= self.max_depth) | ||||||
|
||||||
def write_packages(self, diagram: PackageDiagram) -> None: | ||||||
"""Write a package diagram.""" | ||||||
module_info: dict[str, dict[str, int]] = {} | ||||||
|
@@ -61,6 +94,10 @@ | |||||
for module in sorted(diagram.modules(), key=lambda x: x.title): | ||||||
module.fig_id = module.node.qname() | ||||||
|
||||||
# Filter nodes based on depth | ||||||
if not self.should_show_node(module.fig_id): | ||||||
continue | ||||||
|
||||||
if self.config.no_standalone and not any( | ||||||
module in (rel.from_object, rel.to_object) | ||||||
for rel in diagram.get_relationships("depends") | ||||||
|
@@ -83,6 +120,10 @@ | |||||
from_id = rel.from_object.fig_id | ||||||
to_id = rel.to_object.fig_id | ||||||
|
||||||
# Filter nodes based on depth ==> skip if either source or target nodes is beyond the max depth | ||||||
if not self.should_show_node(from_id) or not self.should_show_node(to_id): | ||||||
continue | ||||||
|
||||||
self.printer.emit_edge( | ||||||
from_id, | ||||||
to_id, | ||||||
|
@@ -96,6 +137,10 @@ | |||||
from_id = rel.from_object.fig_id | ||||||
to_id = rel.to_object.fig_id | ||||||
|
||||||
# Filter nodes based on depth ==> skip if either source or target nodes is beyond the max depth | ||||||
if not self.should_show_node(from_id) or not self.should_show_node(to_id): | ||||||
continue | ||||||
|
||||||
self.printer.emit_edge( | ||||||
from_id, | ||||||
to_id, | ||||||
|
@@ -115,6 +160,11 @@ | |||||
# sorted to get predictable (hence testable) results | ||||||
for obj in sorted(diagram.objects, key=lambda x: x.title): | ||||||
obj.fig_id = obj.node.qname() | ||||||
|
||||||
# Filter class based on depth setting | ||||||
if not self.should_show_node(obj.fig_id, is_class=True): | ||||||
continue | ||||||
|
||||||
if self.config.no_standalone and not any( | ||||||
obj in (rel.from_object, rel.to_object) | ||||||
for rel_type in ("specialization", "association", "aggregation") | ||||||
|
@@ -129,6 +179,12 @@ | |||||
) | ||||||
# inheritance links | ||||||
for rel in diagram.get_relationships("specialization"): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Wonder if it's possible of filtering on depth directly in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I feel like we could simplify this, by introducing 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks great ! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a big change, I am not sure how to approach this. Are you ok with changing it in this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, let's open another PR for the refactor required to add the depth argument. |
||||||
# Filter nodes based on depth setting | ||||||
if not self.should_show_node( | ||||||
rel.from_object.fig_id, is_class=True | ||||||
) or not self.should_show_node(rel.to_object.fig_id, is_class=True): | ||||||
continue | ||||||
|
||||||
self.printer.emit_edge( | ||||||
rel.from_object.fig_id, | ||||||
rel.to_object.fig_id, | ||||||
|
@@ -137,6 +193,12 @@ | |||||
associations: dict[str, set[str]] = defaultdict(set) | ||||||
# generate associations | ||||||
for rel in diagram.get_relationships("association"): | ||||||
# Filter nodes based on depth setting | ||||||
if not self.should_show_node( | ||||||
rel.from_object.fig_id, is_class=True | ||||||
) or not self.should_show_node(rel.to_object.fig_id, is_class=True): | ||||||
continue | ||||||
|
||||||
associations[rel.from_object.fig_id].add(rel.to_object.fig_id) | ||||||
self.printer.emit_edge( | ||||||
rel.from_object.fig_id, | ||||||
|
@@ -146,6 +208,12 @@ | |||||
) | ||||||
# generate aggregations | ||||||
for rel in diagram.get_relationships("aggregation"): | ||||||
# Filter nodes based on depth setting | ||||||
if not self.should_show_node( | ||||||
rel.from_object.fig_id, is_class=True | ||||||
) or not self.should_show_node(rel.to_object.fig_id, is_class=True): | ||||||
continue | ||||||
|
||||||
if rel.to_object.fig_id in associations[rel.from_object.fig_id]: | ||||||
continue | ||||||
self.printer.emit_edge( | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
digraph "classes_depth_limited_0" { | ||
rankdir=BT | ||
charset="utf-8" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
digraph "classes_depth_limited_1" { | ||
rankdir=BT | ||
charset="utf-8" | ||
"data.clientmodule_test.Ancestor" [color="black", fontcolor="black", label=<{Ancestor|attr : str<br ALIGN="LEFT"/>cls_member<br ALIGN="LEFT"/>|get_value()<br ALIGN="LEFT"/>set_value(value)<br ALIGN="LEFT"/>}>, shape="record", style="solid"]; | ||
"data.suppliermodule_test.CustomException" [color="black", fontcolor="red", label=<{CustomException|<br ALIGN="LEFT"/>|}>, shape="record", style="solid"]; | ||
"data.suppliermodule_test.DoNothing" [color="black", fontcolor="black", label=<{DoNothing|<br ALIGN="LEFT"/>|}>, shape="record", style="solid"]; | ||
"data.suppliermodule_test.DoNothing2" [color="black", fontcolor="black", label=<{DoNothing2|<br ALIGN="LEFT"/>|}>, shape="record", style="solid"]; | ||
"data.suppliermodule_test.DoSomething" [color="black", fontcolor="black", label=<{DoSomething|my_int : Optional[int]<br ALIGN="LEFT"/>my_int_2 : Optional[int]<br ALIGN="LEFT"/>my_string : str<br ALIGN="LEFT"/>|do_it(new_int: int): int<br ALIGN="LEFT"/>}>, shape="record", style="solid"]; | ||
"data.suppliermodule_test.Interface" [color="black", fontcolor="black", label=<{Interface|<br ALIGN="LEFT"/>|<I>get_value</I>()<br ALIGN="LEFT"/><I>set_value</I>(value)<br ALIGN="LEFT"/>}>, shape="record", style="solid"]; | ||
"data.nullable_pattern.NullablePatterns" [color="black", fontcolor="black", label=<{NullablePatterns|<br ALIGN="LEFT"/>|<I>return_nullable_1</I>(): int \| None<br ALIGN="LEFT"/><I>return_nullable_2</I>(): Optional[int]<br ALIGN="LEFT"/>}>, shape="record", style="solid"]; | ||
"data.property_pattern.PropertyPatterns" [color="black", fontcolor="black", label=<{PropertyPatterns|prop1<br ALIGN="LEFT"/>prop2<br ALIGN="LEFT"/>|}>, shape="record", style="solid"]; | ||
"data.clientmodule_test.Specialization" [color="black", fontcolor="black", label=<{Specialization|TYPE : str<br ALIGN="LEFT"/>relation<br ALIGN="LEFT"/>relation2<br ALIGN="LEFT"/>top : str<br ALIGN="LEFT"/>|from_value(value: int)<br ALIGN="LEFT"/>increment_value(): None<br ALIGN="LEFT"/>transform_value(value: int): int<br ALIGN="LEFT"/>}>, shape="record", style="solid"]; | ||
"data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"]; | ||
"data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Ancestor" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="cls_member", style="solid"]; | ||
"data.suppliermodule_test.DoNothing" -> "data.clientmodule_test.Specialization" [arrowhead="diamond", arrowtail="none", fontcolor="green", label="relation", style="solid"]; | ||
"data.suppliermodule_test.DoNothing2" -> "data.clientmodule_test.Specialization" [arrowhead="odiamond", arrowtail="none", fontcolor="green", label="relation2", style="solid"]; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
digraph "packages_depth_limited_0" { | ||
rankdir=BT | ||
charset="utf-8" | ||
"data" [color="black", label=<data>, shape="box", style="solid"]; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
digraph "packages_depth_limited_1" { | ||
rankdir=BT | ||
charset="utf-8" | ||
"data" [color="black", label=<data>, shape="box", style="solid"]; | ||
"data.clientmodule_test" [color="black", label=<data.clientmodule_test>, shape="box", style="solid"]; | ||
"data.nullable_pattern" [color="black", label=<data.nullable_pattern>, shape="box", style="solid"]; | ||
"data.property_pattern" [color="black", label=<data.property_pattern>, shape="box", style="solid"]; | ||
"data.suppliermodule_test" [color="black", label=<data.suppliermodule_test>, shape="box", style="solid"]; | ||
"data.clientmodule_test" -> "data.suppliermodule_test" [arrowhead="open", arrowtail="none"]; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.