-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add new animation TraceColor
(Name for debate) and Graph.set_edge_color()
#3533
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
base: main
Are you sure you want to change the base?
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.
I like the animation idea!
Left some comments and questions on the implementation. Additionally, it would be nice to add some tests to make sure the methods are working properly.
""" | ||
|
||
def __init__( | ||
self, mobject: "VMobject", color: ParsableManimColor, **kwargs |
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.
Is there a reason VMobject
is in quotations?
@@ -151,6 +151,7 @@ def __init__( | |||
self.suspend_mobject_updating: bool = suspend_mobject_updating | |||
self.lag_ratio: float = lag_ratio | |||
self._on_finish: Callable[[Scene], None] = _on_finish | |||
self.mobject: VMobject |
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.
self.mobject: VMobject |
Is this neccessary?
anim_args = {} | ||
animation = anim_args.pop("animation", TraceColor) | ||
|
||
mobj = self.get_edge(vertices) # Will error on DiGraph if edge is wrong way |
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.
Might be a good idea to add a warning either in the code (via logger.warning
) or in the docstring about DiGraph
.
Same for the part below saying "this will not work on DiGraph"
The order of the vertices is relevant here. | ||
""" | ||
try: | ||
return self._edges[edge] |
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.
return self._edges[edge] | |
return self.edges[edge] |
Typo?
@@ -584,7 +587,7 @@ def __init__( | |||
edge_config: dict | None = None, | |||
) -> None: | |||
super().__init__() | |||
|
|||
self.edges = {} |
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.
self.edges = {} | |
self.edges: dict[tuple[Hashable, Hashable], Mobject] = {} |
This way linters can raise errors about setting/getting stuff from this attribute.
This PR adds some functionality which should simplify the usage of Graphs and animating the edges.
get_edge()
now returns the Edge no matter if it is flipped or not. But only forGraph
not forDiGraph
TraceColor
now exists which can smoothly transition from one color to another over a shapeset_edge_color
on GenericGraph is overriden with an Animation such that it produces aTraceColor
effect when animating edge colors.SetEdgeColor.mp4
Where
TraceColor
itself has the following behaviorTraceColorExample.mp4
There are still some problems with the z_indices of objects in the graph but the concept works