From db1931d7f5df79b801064a866b51cfc8f3b8389d Mon Sep 17 00:00:00 2001 From: Hugues Devimeux Date: Tue, 8 Sep 2020 15:09:33 +0200 Subject: [PATCH 1/5] fixes n flag --- manim/config/config_utils.py | 4 +--- manim/scene/scene.py | 16 +++++++++++----- manim/scene/scene_file_writer.py | 4 +++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/manim/config/config_utils.py b/manim/config/config_utils.py index 37b265032a..5d81eac6a3 100644 --- a/manim/config/config_utils.py +++ b/manim/config/config_utils.py @@ -147,9 +147,7 @@ def _parse_file_writer_config(config_parser, args): } # For internal use (no CLI flag) - fw_config["skip_animations"] = any( - [fw_config["save_last_frame"], fw_config["from_animation_number"]] - ) + fw_config["skip_animations"] = fw_config["save_last_frame"] fw_config["max_files_cached"] = default.getint("max_files_cached") if fw_config["max_files_cached"] == -1: fw_config["max_files_cached"] = float("inf") diff --git a/manim/scene/scene.py b/manim/scene/scene.py index 1ca43e09a1..4fd665f4db 100644 --- a/manim/scene/scene.py +++ b/manim/scene/scene.py @@ -68,6 +68,7 @@ def __init__(self, **kwargs): self.file_writer = SceneFileWriter(self, **file_writer_config,) self.play_hashes_list = [] self.mobjects = [] + self.original_skipping_status = file_writer_config["skip_animations"] # TODO, remove need for foreground mobjects self.foreground_mobjects = [] self.num_plays = 0 @@ -85,7 +86,7 @@ def __init__(self, **kwargs): self.tear_down() # We have to reset these settings in case of multiple renders. file_writer_config["skip_animations"] = False - self.original_skipping_status = file_writer_config["skip_animations"] + self.file_writer.finish() self.print_end_message() @@ -779,10 +780,10 @@ def update_skipping_status(self): """ if file_writer_config["from_animation_number"]: - if self.num_plays == file_writer_config["from_animation_number"]: - file_writer_config["skip_animations"] = False + if self.num_plays < file_writer_config["from_animation_number"]: + file_writer_config["skip_animations"] = True if file_writer_config["upto_animation_number"]: - if self.num_plays >= file_writer_config["upto_animation_number"]: + if self.num_plays > file_writer_config["upto_animation_number"]: file_writer_config["skip_animations"] = True raise EndSceneEarlyException() @@ -800,8 +801,13 @@ def handle_caching_play(func): def wrapper(self, *args, **kwargs): self.revert_to_original_skipping_status() + self.update_skipping_status() animations = self.compile_play_args_to_animation_list(*args, **kwargs) self.add_mobjects_from_animations(animations) + if file_writer_config["skip_animations"]: + logger.debug(f"Skipping animation {self.num_plays}") + func(self, *args, **kwargs) + return if not file_writer_config["disable_caching"]: mobjects_on_scene = self.get_mobjects() hash_play = get_hash_from_play_call( @@ -834,6 +840,7 @@ def handle_caching_wait(func): def wrapper(self, duration=DEFAULT_WAIT_TIME, stop_condition=None): self.revert_to_original_skipping_status() + self.update_skipping_status() if not file_writer_config["disable_caching"]: hash_wait = get_hash_from_wait_call( self.camera, duration, stop_condition, self.get_mobjects() @@ -873,7 +880,6 @@ def handle_play_like_call(func): """ def wrapper(self, *args, **kwargs): - self.update_skipping_status() allow_write = not file_writer_config["skip_animations"] self.file_writer.begin_animation(allow_write) func(self, *args, **kwargs) diff --git a/manim/scene/scene_file_writer.py b/manim/scene/scene_file_writer.py index eb9e8b7289..6913c5f6a6 100644 --- a/manim/scene/scene_file_writer.py +++ b/manim/scene/scene_file_writer.py @@ -44,6 +44,7 @@ def __init__(self, scene, **kwargs): self.init_output_directories() self.init_audio() self.frame_count = 0 + self.index_partial_movie_file = 0 # Output directories and files def init_output_directories(self): @@ -191,10 +192,11 @@ def get_next_partial_movie_path(self): result = os.path.join( self.partial_movie_directory, "{}{}".format( - self.scene.play_hashes_list[self.scene.num_plays], + self.scene.play_hashes_list[self.index_partial_movie_file], file_writer_config["movie_file_extension"], ), ) + self.index_partial_movie_file += 1 return result def get_movie_file_path(self): From e8e762e7f6a780fbc83677a517e0c643f85c2e8b Mon Sep 17 00:00:00 2001 From: Hugues Devimeux Date: Tue, 8 Sep 2020 15:32:44 +0200 Subject: [PATCH 2/5] added test for n flag --- .../SceneWithMultipleCallsWithNFlag.json | 11 +++ tests/helpers/video_utils.py | 8 +- tests/test_scene_rendering/simple_scenes.py | 9 ++ tests/utils/logging_tester.py | 88 +++++++++++++++++++ 4 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/control_data/videos_data/SceneWithMultipleCallsWithNFlag.json create mode 100644 tests/utils/logging_tester.py diff --git a/tests/control_data/videos_data/SceneWithMultipleCallsWithNFlag.json b/tests/control_data/videos_data/SceneWithMultipleCallsWithNFlag.json new file mode 100644 index 0000000000..2e125f125c --- /dev/null +++ b/tests/control_data/videos_data/SceneWithMultipleCallsWithNFlag.json @@ -0,0 +1,11 @@ +{ + "name": "SceneWithMultipleCallsWithNFlag", + "config": { + "codec_name": "h264", + "width": 1920, + "height": 1080, + "avg_frame_rate": "60/1", + "duration": "4.000000", + "nb_frames": "240" + } +} \ No newline at end of file diff --git a/tests/helpers/video_utils.py b/tests/helpers/video_utils.py index 4d3058b1cd..a59f3fa9ec 100644 --- a/tests/helpers/video_utils.py +++ b/tests/helpers/video_utils.py @@ -4,7 +4,7 @@ import subprocess import json -from manim.logger import logger +from manim.config.logger import logger def capture(command): @@ -57,3 +57,9 @@ def save_control_data_from_video(path_to_video, name): path_saved = os.path.join(path_control_data, f"{name}.json") json.dump(data, open(path_saved, "w"), indent=4) logger.info(f"Data for {name} saved in {path_saved}") + + +save_control_data_from_video( + r"/home/hugues/Desktop/Programmation/PYTHON/MANIM-DEV/media/videos/test/1080p60/SceneWithMultipleCalls.mp4", + "SceneWithMultipleCallsWithNFlag", +) diff --git a/tests/test_scene_rendering/simple_scenes.py b/tests/test_scene_rendering/simple_scenes.py index 1b65ea1173..5bb905e8a4 100644 --- a/tests/test_scene_rendering/simple_scenes.py +++ b/tests/test_scene_rendering/simple_scenes.py @@ -6,3 +6,12 @@ def construct(self): square = Square() circle = Circle() self.play(Transform(square, circle)) + + +class SceneWithMultipleCalls(Scene): + def construct(self): + number = Integer(0) + self.add(number) + for i in range(10): + number.become(Integer(i)) + self.play(Animation(number)) \ No newline at end of file diff --git a/tests/utils/logging_tester.py b/tests/utils/logging_tester.py new file mode 100644 index 0000000000..051ac41d0e --- /dev/null +++ b/tests/utils/logging_tester.py @@ -0,0 +1,88 @@ +from functools import wraps +import os +import json +import itertools + + +def _check_logs(reference_logfile, generated_logfile): + with open(reference_logfile, "r") as reference_logs, open( + generated_logfile, "r" + ) as generated_logs: + reference_logs = reference_logs.readlines() + generated_logs = generated_logs.readlines() + diff = abs(len(reference_logs) - len(generated_logs)) + if len(reference_logs) != len(generated_logs): + msg_assert = "" + if len(reference_logs) > len(generated_logs): + msg_assert += f"Logs generated are SHORTER than the expected logs. There are {diff} extra logs.\n" + msg_assert += "Last log of the generated log is : \n" + msg_assert += generated_logs[-1] + else: + msg_assert += f"Logs generated are LONGER than the expected logs.\n There are {diff} extra logs :\n" + for log in generated_logs[len(reference_logs) :]: + msg_assert += log + assert 0, msg_assert + + for index, ref, gen in zip(itertools.count(), reference_logs, generated_logs): + # As they are string, we only need to check if they are equal. If they are not, we then compute a more precise difference, to debug. + if ref == gen: + continue + ref_log = json.loads(ref) + gen_log = json.loads(gen) + diff_keys = [ + d1[0] for d1, d2 in zip(ref_log.items(), gen_log.items()) if d1[1] != d2[1] + ] + # \n and \t don't not work in f-strings. + newline = "\n" + tab = "\t" + assert ( + len(diff_keys) == 0 + ), f"Lgos don't match at {index} st log. : \n{newline.join([f'In {key} field, got -> {newline}{tab}{repr(gen_log[key])}. {newline}Expected : -> {newline}{tab}{repr(ref_log[key])}.' for key in diff_keys])}" + + +def logs_comparison(control_data_file, log_path_from_media_dir): + """Decorator used for any test that needs to check logs. + + Parameters + ---------- + control_data_file : :class:`str` + Name of the control data file, i.e. .log that will be compared to the outputed logs. + .. warning:: You don't have to pass the path here. + .. example:: "SquareToCircleWithLFlag.log" + + log_path_from_media_dir : :class:`str` + The path of the .log generated, from the media dir. Example: /logs/Square.log. + + Returns + ------- + Callable[[Any], Any] + The test wrapped with which we are going to make the comparison. + """ + + def decorator(f): + @wraps(f) + def wrapper(*args, **kwargs): + # NOTE : Every args goes seemingly in kwargs instead of args; this is perhaps Pytest. + result = f(*args, **kwargs) + tmp_path = kwargs["tmp_path"] + tests_directory = os.path.dirname( + os.path.dirname(os.path.abspath(__file__)) + ) + controle_data_path = os.path.join( + tests_directory, "control_data", "logs_data", control_data_file + ) + path_log_generated = tmp_path / log_path_from_media_dir + # The following will say precisely which subdir does not exist. + if not os.path.exists(path_log_generated): + for parent in reversed(path_log_generated.parents): + if not parent.exists(): + assert ( + False + ), f"'{parent.name}' does not exist in '{parent.parent}' (which exists). " + break + _check_logs(controle_data_path, str(path_log_generated)) + return result + + return wrapper + + return decorator From 45aaf24ef85d1ae11041ebed9bc4eeb69cb0ac41 Mon Sep 17 00:00:00 2001 From: Hugues Devimeux Date: Tue, 8 Sep 2020 15:46:19 +0200 Subject: [PATCH 3/5] mistake --- tests/helpers/video_utils.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/helpers/video_utils.py b/tests/helpers/video_utils.py index a59f3fa9ec..5b7aa2791f 100644 --- a/tests/helpers/video_utils.py +++ b/tests/helpers/video_utils.py @@ -57,9 +57,3 @@ def save_control_data_from_video(path_to_video, name): path_saved = os.path.join(path_control_data, f"{name}.json") json.dump(data, open(path_saved, "w"), indent=4) logger.info(f"Data for {name} saved in {path_saved}") - - -save_control_data_from_video( - r"/home/hugues/Desktop/Programmation/PYTHON/MANIM-DEV/media/videos/test/1080p60/SceneWithMultipleCalls.mp4", - "SceneWithMultipleCallsWithNFlag", -) From 6ed3c43a8f50748ea0aa8a9c2a15754448543169 Mon Sep 17 00:00:00 2001 From: Hugues Devimeux Date: Tue, 8 Sep 2020 23:57:14 +0200 Subject: [PATCH 4/5] punished git, so he doesn't do crazy things any more --- tests/test_scene_rendering/test_cli_flags.py | 21 +++++ tests/utils/logging_tester.py | 88 -------------------- 2 files changed, 21 insertions(+), 88 deletions(-) delete mode 100644 tests/utils/logging_tester.py diff --git a/tests/test_scene_rendering/test_cli_flags.py b/tests/test_scene_rendering/test_cli_flags.py index 3588a38bea..307b00a5dc 100644 --- a/tests/test_scene_rendering/test_cli_flags.py +++ b/tests/test_scene_rendering/test_cli_flags.py @@ -44,3 +44,24 @@ def test_basic_scene_l_flag(tmp_path, manim_cfg_file, simple_scenes_path): ] out, err, exit_code = capture(command) assert exit_code == 0, err + + +@pytest.mark.slow +@video_comparison( + "SceneWithMultipleCallsWithNFlag.json", + "videos/simple_scenes/1080p60/SceneWithMultipleCalls.mp4", +) +def test_n_flag(tmp_path, simple_scenes_path): + scene_name = "SceneWithMultipleCalls" + command = [ + "python", + "-m", + "manim", + simple_scenes_path, + scene_name, + "-n 3,6", + "--media_dir", + str(tmp_path), + ] + _, err, exit_code = capture(command) + assert exit_code == 0, err \ No newline at end of file diff --git a/tests/utils/logging_tester.py b/tests/utils/logging_tester.py deleted file mode 100644 index 051ac41d0e..0000000000 --- a/tests/utils/logging_tester.py +++ /dev/null @@ -1,88 +0,0 @@ -from functools import wraps -import os -import json -import itertools - - -def _check_logs(reference_logfile, generated_logfile): - with open(reference_logfile, "r") as reference_logs, open( - generated_logfile, "r" - ) as generated_logs: - reference_logs = reference_logs.readlines() - generated_logs = generated_logs.readlines() - diff = abs(len(reference_logs) - len(generated_logs)) - if len(reference_logs) != len(generated_logs): - msg_assert = "" - if len(reference_logs) > len(generated_logs): - msg_assert += f"Logs generated are SHORTER than the expected logs. There are {diff} extra logs.\n" - msg_assert += "Last log of the generated log is : \n" - msg_assert += generated_logs[-1] - else: - msg_assert += f"Logs generated are LONGER than the expected logs.\n There are {diff} extra logs :\n" - for log in generated_logs[len(reference_logs) :]: - msg_assert += log - assert 0, msg_assert - - for index, ref, gen in zip(itertools.count(), reference_logs, generated_logs): - # As they are string, we only need to check if they are equal. If they are not, we then compute a more precise difference, to debug. - if ref == gen: - continue - ref_log = json.loads(ref) - gen_log = json.loads(gen) - diff_keys = [ - d1[0] for d1, d2 in zip(ref_log.items(), gen_log.items()) if d1[1] != d2[1] - ] - # \n and \t don't not work in f-strings. - newline = "\n" - tab = "\t" - assert ( - len(diff_keys) == 0 - ), f"Lgos don't match at {index} st log. : \n{newline.join([f'In {key} field, got -> {newline}{tab}{repr(gen_log[key])}. {newline}Expected : -> {newline}{tab}{repr(ref_log[key])}.' for key in diff_keys])}" - - -def logs_comparison(control_data_file, log_path_from_media_dir): - """Decorator used for any test that needs to check logs. - - Parameters - ---------- - control_data_file : :class:`str` - Name of the control data file, i.e. .log that will be compared to the outputed logs. - .. warning:: You don't have to pass the path here. - .. example:: "SquareToCircleWithLFlag.log" - - log_path_from_media_dir : :class:`str` - The path of the .log generated, from the media dir. Example: /logs/Square.log. - - Returns - ------- - Callable[[Any], Any] - The test wrapped with which we are going to make the comparison. - """ - - def decorator(f): - @wraps(f) - def wrapper(*args, **kwargs): - # NOTE : Every args goes seemingly in kwargs instead of args; this is perhaps Pytest. - result = f(*args, **kwargs) - tmp_path = kwargs["tmp_path"] - tests_directory = os.path.dirname( - os.path.dirname(os.path.abspath(__file__)) - ) - controle_data_path = os.path.join( - tests_directory, "control_data", "logs_data", control_data_file - ) - path_log_generated = tmp_path / log_path_from_media_dir - # The following will say precisely which subdir does not exist. - if not os.path.exists(path_log_generated): - for parent in reversed(path_log_generated.parents): - if not parent.exists(): - assert ( - False - ), f"'{parent.name}' does not exist in '{parent.parent}' (which exists). " - break - _check_logs(controle_data_path, str(path_log_generated)) - return result - - return wrapper - - return decorator From 26f7cfb4a44366292af8065c1f47bfaf6f9f5b33 Mon Sep 17 00:00:00 2001 From: Hugues Devimeux <36239975+huguesdevimeux@users.noreply.github.com> Date: Mon, 14 Sep 2020 19:25:51 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Aathish Sivasubrahmanian --- tests/test_scene_rendering/simple_scenes.py | 2 +- tests/test_scene_rendering/test_cli_flags.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_scene_rendering/simple_scenes.py b/tests/test_scene_rendering/simple_scenes.py index 5bb905e8a4..64e6b4f420 100644 --- a/tests/test_scene_rendering/simple_scenes.py +++ b/tests/test_scene_rendering/simple_scenes.py @@ -14,4 +14,4 @@ def construct(self): self.add(number) for i in range(10): number.become(Integer(i)) - self.play(Animation(number)) \ No newline at end of file + self.play(Animation(number)) diff --git a/tests/test_scene_rendering/test_cli_flags.py b/tests/test_scene_rendering/test_cli_flags.py index 307b00a5dc..525a6fbe7b 100644 --- a/tests/test_scene_rendering/test_cli_flags.py +++ b/tests/test_scene_rendering/test_cli_flags.py @@ -64,4 +64,4 @@ def test_n_flag(tmp_path, simple_scenes_path): str(tmp_path), ] _, err, exit_code = capture(command) - assert exit_code == 0, err \ No newline at end of file + assert exit_code == 0, err