-
Notifications
You must be signed in to change notification settings - Fork 192
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
🔧 Type annotate aiida/tools/visualization/graph.py
#5821
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chrisjsewell . Just two comments
aiida/tools/visualization/graph.py
Outdated
@@ -27,18 +28,22 @@ | |||
|
|||
__all__ = ('Graph', 'default_link_styles', 'default_node_styles', 'pstate_node_styles', 'default_node_sublabels') | |||
|
|||
LinkAnnotateType = Literal[None, False, 'label', 'type', 'both'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to accept both None
and False
? I think either one should suffice and probably just get rid of None
and then change the default for annotate_links
in add_incoming
and add_outgoing
to False
instead of None
. All other functions already have False
as default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
aiida/tools/visualization/graph.py
Outdated
@@ -405,69 +400,48 @@ def __init__( | |||
self._node_id_type = node_id_type | |||
self._backend = backend or get_manager().get_profile_storage() | |||
|
|||
self._ignore_node_style = _OVERRIDE_STYLES_DICT['ignore_node'] | |||
self._origin_node_style = _OVERRIDE_STYLES_DICT['origin_node'] | |||
self._ignore_node_style = _OVERRIDE_STYLES_DICT['ignore_node'].copy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of putting the responsibility on the caller of passing a clone to add_node
just have add_node
create a copy
(maybe a deepcopy
in case in the future the keys of _OVERRIDE_STYLES_DICT
become nested). This will reduce the risk of a user forgetting to create a copy when calling add_node
manually. Example is line 823 where the dictionary passed in is not copied and will be modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just converted _OVERRIDE_STYLES_DICT
to a function, so its no longer mutable
aiida/tools/visualization/graph.py
No description provided.