Adding docs for VectorScene and LinearTransformationScene and reimplementing method #3681
Conversation
…, more information in PR comments
for more information, see https://pre-commit.ci
| @@ -79,20 +79,90 @@ def add_plane(self, animate: bool = False, **kwargs): | |||
| self.add(plane) | |||
| return plane | |||
|
|
|||
| def add_axes(self, animate: bool = False, color: bool = WHITE, **kwargs): | |||
| def add_axes( | |||
There was a problem hiding this comment.
Why did I remove the color argument?
Because it is not working.
self.add_axes(color=RED) will not work, because the color of the VGroup is set before the axes creation.
I could add these lines:
if color:
axes.set_color(color)but I don't see the reason, the much more powerful would be the new solution when a user could use dictionaries to specify the exact all parameters in axes
| def add_axes( | ||
| self, | ||
| animate: bool = False, | ||
| animation: Animation = Create, |
There was a problem hiding this comment.
animation, animation_kwargs and play_kwargs
Why must a user be limited to the Create animation? -> Solution: my changes
It would annoy me if I wanted to create a more advanced animation but I would have only Create animation available.
| self.wait() | ||
|
|
||
| """ | ||
| axis_kwargs = merge_dicts_recursively( |
There was a problem hiding this comment.
This could be marked in docs that there are default parameters for axis settings, and to change it the user have to pass it explicitly
There was a problem hiding this comment.
Instead of adding lots of options for a simple animation, it would be better to just deprecate the method and have the user animate it themselves. EDIT: my mistake, I misunderstood the methods. If you want to do any custom animations, you should simply pass animate=False and do it yourself.
Also, please do not add the changes from #3675 here: that is a separate PR for a reason (git blame, if you're curious).
| animation_kwargs: dict[str, Any] | None = {}, | ||
| play_kwargs: dict[str, Any] | None = {}, | ||
| axis_config: dict[str, Any] | None = {}, | ||
| **axis_kwargs: dict[str, Any] | None, |
There was a problem hiding this comment.
| animation_kwargs: dict[str, Any] | None = {}, | |
| play_kwargs: dict[str, Any] | None = {}, | |
| axis_config: dict[str, Any] | None = {}, | |
| **axis_kwargs: dict[str, Any] | None, | |
| animation_kwargs: dict[str, Any] = {}, | |
| play_kwargs: dict[str, Any] = {}, | |
| axis_config: dict[str, Any] = {}, | |
| **axis_kwargs: Any, |
These are the correct typehints, as your code does not deal with the case of each parameter being None.
In addition to that, **kwargs is always typehinted as the type of the value of the dict.
There was a problem hiding this comment.
So, I should identify the method of depreciation, remove them, and add examples for that, which will not be deprecated
There was a problem hiding this comment.
I would remove the following:
add_planeadd_axesadd_vectorposition_x_coordinateposition_y_coordinate
and in LinearTransformationScene
I would move the content of the add_vector (from the base class) into the body of the add_vector in the derived class.
If I understand, please let me know if I am wrong. Does the depreciation method mean deleting them from the source code? Right?
There was a problem hiding this comment.
After taking a look at the source code again, I will say that it's my mistake. These methods shouldn't be deprecated.
Instead of adding lots of parameters, the user should simply pass animate=False and then animate it themselves.
There was a problem hiding this comment.
Does this mean I should create as many code examples as possible?
Please explain a little bit about self.moving_mobjects member variable? What is this responsible for?
There was a problem hiding this comment.
This means the implementation of the methods should be the same.
As for code examples, something like the following would suffice:
>>> from manim import LinearTransformationScene
>>> scene = LinearTransformationScene()
>>> scene.add_axes()
Axes(NumberLine, NumberLine)…amesClarkMaxwell/manim into VectorScene-fixingBugs-docs
JasonGrace2282
added
the
documentation
|
Closing due to inactivity. |
…, more information in PR comments
Overview: What does this pull request change?
Motivation and Explanation: Why and how do your changes improve the library?
When I worked in #3479 I noticed no examples for VectorScene and LinearTransformationScene, so I decided to change it.
During the writing example of the add_axes method, I noticed some bugs and its usage was quite limited. I decided to change it. I will add the reason for all changes in separate comments, in concrete changes.
Links to added or changed documentation pages
Further Information and Comments
please ignore the changes in
get_pieces_movementI will remove them in the next commit, It was attached because I copied the code from another branch and forgot that they were there.Reviewer Checklist