Fix n flag #423
Fix n flag #423
Conversation
huguesdevimeux
added
pr:bugfix
leotrs
left a comment
leotrs
left a comment
There was a problem hiding this comment.
Thanks! Question tho: you mention you added a test to test_cli_flags, but I don't see that file in the diff.
| fw_config["skip_animations"] = any( | ||
| [fw_config["save_last_frame"], fw_config["from_animation_number"]] | ||
| ) | ||
| fw_config["skip_animations"] = fw_config["save_last_frame"] |
There was a problem hiding this comment.
Question: can you explain why this change is necessary?
There was a problem hiding this comment.
Because, setting skip_animation to True here because of from_animation_number here does not make any sense, as skip_animation is set in consequence of this flag already in update_skipping_status,
Furthermore, (this is a particular case) when one is trying to start from the Animation 0 with -n 0,2 manim will skip the first animation as skip_animations would have been set to True here.
| pass | ||
| self.tear_down() | ||
| # We have to reset these settings in case of multiple renders. | ||
| file_writer_config["skip_animations"] = False |
There was a problem hiding this comment.
Issue: It should be reset to self.original_skipping_status, not to False.
There was a problem hiding this comment.
Hmm fair point, I disagree. scene.original_skipping_status value is set to the one of file_writer_config[skip_animations] at the beginning of __init__, so it will be set to False in the next scene instantiation.
My concern is when scene.original_skipping_status has been changed for a reason or another. If it is the case and we do your suggestion, then scene.original_skipping_status will never be set to False again, for the reason above.
| """ | ||
|
|
||
| def wrapper(self, *args, **kwargs): | ||
| self.update_skipping_status() |
There was a problem hiding this comment.
Can you please explain why it is not necessary here to update the skipping status?
There was a problem hiding this comment.
Because now it will be done before, in handle_caching_play or handle_caching_wait !
@leotrs THis is weird. Git is doing crazy things. Nvm, I fixed that :D |
Co-authored-by: Aathish Sivasubrahmanian <[email protected]>
Thanks ! |
3 participants
List of Changes
Explanation for Changes
I modified the mechanism behind -n flag. Before, manim rendered all the partial movie files and selected only the ones with the index between i and j (where i and j are integreers passed with -n flag, eg
-n 5,6).Now, it skips the entire process if an animation is outside the [i,j] interval and creates partial movies files otherwise.
Testing Status
Created a test for n flag in
test_scene_rendering/test_cli_flags.py