-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Add classes MethodWithArgs
, SceneInteractContinue
and SceneInteractRerun
inside new module manim.data_structures
#4315
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
MethodWithArgs
, SceneInteractRerun
and SceneInteractExit
inside new module manim.data_structures
MethodWithArgs
, SceneInteractContinue
and SceneInteractRerun
inside new module manim.data_structures
Note: it seems only |
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.
This seems like a fine improvement and cleanup -- could you please just resolve the conflict / move the things you wanted to move first and then let me know (just re-request a review or ping me on Discord).
…ata_structures
|
||
kwargs = tup[2] | ||
if "from_animation_number" in kwargs: | ||
config["from_animation_number"] = kwargs[ | ||
if "from_animation_number" in action.kwargs: | ||
config["from_animation_number"] = action.kwargs[ |
Check notice
Code scanning / CodeQL
Commented-out code Note
I just did, and I also added the Note: the docstring of |
Overview: What does this pull request change?
MethodWithArgs
,SceneInteractContinue
andSceneInteractRerun
. Currently, they all exist inside a newmanim.data_structures
file, although I intend to moveSceneInteractContinue
andSceneInteractRerun
intoscene.py
if PR Moveconfigure_pygui
into aScene
method and removemanim.gui
#4314 is merged first.Scene.queue
,_AnimationBuilder.methods
and_MethodAnimation.methods
objects of these classes instead of tuples.Motivation and Explanation: Why and how do your changes improve the library?
Scene.queue
,_AnimationBuilder.methods
and_MethodAnimation.methods
contain tuples where the first element represents a method to be called, the second one is its args and the third one is its kwargs. Moreover, in the case ofScene.queue
, the first element (a string) might not necessarily represent a method: it can be a key starting with "rerun" (in which case the args are not used and only some key-value pairs are extracted from the kwargs) or with "exit" (in which case neither the args nor the kwargs are used). On first sight, this might be very confusing. Previously, there were no docs for this behavior and I asked for them to be added in a comment I made while reviewing PR #4260. Now, I believe that it would be better if those were different objects from different classes altogether.Links to added or changed documentation pages
I added
data_structures.py
toutilities_misc.rst
, so now you can see the newMethodWithArgs
structure in:https://manimce--4315.org.readthedocs.build/en/4315/reference/manim.data_structures.html
The
SceneInteractContinue
andSceneInteractRerun
classes are defined in:https://manimce--4315.org.readthedocs.build/en/4315/reference/manim.scene.scene.html
The
SceneInteractAction
type alias was previously defined there as well. However, its docstring is not showing. Presumably, it's because it's indented inside theif TYPE_CHECKING:
clause. It works fine when it's unindented.Further Information and Comments
Reviewer Checklist