-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Conversation
MethodWithArgs, SceneInteractRerun and SceneInteractExit inside new module manim.data_structuresMethodWithArgs, SceneInteractContinue and SceneInteractRerun inside new module manim.data_structures
|
Note: it seems only |
behackl
left a comment
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,SceneInteractContinueandSceneInteractRerun. Currently, they all exist inside a newmanim.data_structuresfile, although I intend to moveSceneInteractContinueandSceneInteractRerunintoscene.pyif PR Moveconfigure_pyguiinto aScenemethod and removemanim.gui#4314 is merged first.Scene.queue,_AnimationBuilder.methodsand_MethodAnimation.methodsobjects of these classes instead of tuples.Motivation and Explanation: Why and how do your changes improve the library?
Scene.queue,_AnimationBuilder.methodsand_MethodAnimation.methodscontain 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.pytoutilities_misc.rst, so now you can see the newMethodWithArgsstructure in:https://manimce--4315.org.readthedocs.build/en/4315/reference/manim.data_structures.html
The
SceneInteractContinueandSceneInteractRerunclasses are defined in:https://manimce--4315.org.readthedocs.build/en/4315/reference/manim.scene.scene.html
The
SceneInteractActiontype 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