From faa1d9d7dbea74168755fba12834b79cd40a75e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= <francisco.manriquezn@usm.cl>
Date: Sat, 26 Oct 2024 14:41:17 -0300
Subject: [PATCH 1/3] Refactored Animation run_time validation

---
 manim/animation/animation.py   | 38 +++++++++++++++++++------
 manim/animation/composition.py |  9 ++++--
 manim/scene/scene.py           | 52 ++++++++++------------------------
 3 files changed, 52 insertions(+), 47 deletions(-)

diff --git a/manim/animation/animation.py b/manim/animation/animation.py
index 8bdf236b3f..c5e80f6e80 100644
--- a/manim/animation/animation.py
+++ b/manim/animation/animation.py
@@ -2,14 +2,13 @@
 
 from __future__ import annotations
 
+from manim._config import config, logger
+from manim.constants import RendererType
+from manim.mobject import mobject
+from manim.mobject.mobject import Mobject
+from manim.mobject.opengl import opengl_mobject
 from manim.mobject.opengl.opengl_mobject import OpenGLMobject
-
-from .. import config, logger
-from ..constants import RendererType
-from ..mobject import mobject
-from ..mobject.mobject import Mobject
-from ..mobject.opengl import opengl_mobject
-from ..utils.rate_functions import linear, smooth
+from manim.utils.rate_functions import linear, smooth
 
 __all__ = ["Animation", "Wait", "override_animation"]
 
@@ -194,6 +193,7 @@ def begin(self) -> None:
         method.
 
         """
+        self.run_time = validate_animation_run_time(self.run_time, str(self))
         self.starting_mobject = self.create_starting_mobject()
         if self.suspend_mobject_updating:
             # All calls to self.mobject's internal updaters
@@ -568,6 +568,28 @@ def prepare_animation(
     raise TypeError(f"Object {anim} cannot be converted to an animation")
 
 
+def validate_animation_run_time(run_time: float, caller_name: str) -> float:
+    if run_time <= 0:
+        raise ValueError(
+            f"{caller_name} has a run_time of <= 0 seconds which Manim cannot render. "
+            "Please set the run_time to be positive."
+        )
+
+    # config.frame_rate holds the number of frames per second
+    frame_rate = 1 / config.frame_rate
+    if run_time < frame_rate:
+        logger.warning(
+            f"Original run time of {caller_name} is shorter than current frame "
+            f"rate (1 frame every {frame_rate:.2f} sec.) which cannot be rendered. "
+            "Rendering with the shortest possible duration instead."
+        )
+        new_run_time = frame_rate
+    else:
+        new_run_time = run_time
+
+    return new_run_time
+
+
 class Wait(Animation):
     """A "no operation" animation.
 
@@ -610,7 +632,7 @@ def __init__(
         self.mobject.shader_wrapper_list = []
 
     def begin(self) -> None:
-        pass
+        self.run_time = validate_animation_run_time(self.run_time, str(self))
 
     def finish(self) -> None:
         pass
diff --git a/manim/animation/composition.py b/manim/animation/composition.py
index c5a756502f..baa7958794 100644
--- a/manim/animation/composition.py
+++ b/manim/animation/composition.py
@@ -9,7 +9,11 @@
 import numpy as np
 
 from manim._config import config
-from manim.animation.animation import Animation, prepare_animation
+from manim.animation.animation import (
+    Animation,
+    prepare_animation,
+    validate_animation_run_time,
+)
 from manim.constants import RendererType
 from manim.mobject.mobject import Group, Mobject
 from manim.mobject.opengl.opengl_mobject import OpenGLGroup
@@ -87,7 +91,7 @@ def begin(self) -> None:
                 f"Trying to play {self} without animations, this is not supported. "
                 "Please add at least one subanimation."
             )
-
+        self.run_time = validate_animation_run_time(self.run_time, str(self))
         self.anim_group_time = 0.0
         if self.suspend_mobject_updating:
             self.group.suspend_updating()
@@ -235,6 +239,7 @@ def begin(self) -> None:
                 f"Trying to play {self} without animations, this is not supported. "
                 "Please add at least one subanimation."
             )
+        self.run_time = validate_animation_run_time(self.run_time, str(self))
         self.update_active_animation(0)
 
     def finish(self) -> None:
diff --git a/manim/scene/scene.py b/manim/scene/scene.py
index 02c548cf7f..ed07fe453e 100644
--- a/manim/scene/scene.py
+++ b/manim/scene/scene.py
@@ -33,23 +33,22 @@
 from watchdog.events import FileSystemEventHandler
 from watchdog.observers import Observer
 
+from manim._config import config, logger
+from manim.animation.animation import Animation, Wait, prepare_animation
+from manim.camera.camera import Camera
+from manim.constants import *
+from manim.gui.gui import configure_pygui
 from manim.mobject.mobject import Mobject
 from manim.mobject.opengl.opengl_mobject import OpenGLPoint
-
-from .. import config, logger
-from ..animation.animation import Animation, Wait, prepare_animation
-from ..camera.camera import Camera
-from ..constants import *
-from ..gui.gui import configure_pygui
-from ..renderer.cairo_renderer import CairoRenderer
-from ..renderer.opengl_renderer import OpenGLRenderer
-from ..renderer.shader import Object3D
-from ..utils import opengl, space_ops
-from ..utils.exceptions import EndSceneEarlyException, RerunSceneException
-from ..utils.family import extract_mobject_family_members
-from ..utils.family_ops import restructure_list_to_exclude_certain_family_members
-from ..utils.file_ops import open_media_file
-from ..utils.iterables import list_difference_update, list_update
+from manim.renderer.cairo_renderer import CairoRenderer
+from manim.renderer.opengl_renderer import OpenGLRenderer
+from manim.renderer.shader import Object3D
+from manim.utils import opengl, space_ops
+from manim.utils.exceptions import EndSceneEarlyException, RerunSceneException
+from manim.utils.family import extract_mobject_family_members
+from manim.utils.family_ops import restructure_list_to_exclude_certain_family_members
+from manim.utils.file_ops import open_media_file
+from manim.utils.iterables import list_difference_update, list_update
 
 if TYPE_CHECKING:
     from collections.abc import Iterable, Sequence
@@ -1030,28 +1029,7 @@ def get_run_time(self, animations: list[Animation]):
         float
             The total ``run_time`` of all of the animations in the list.
         """
-        max_run_time = 0
-        frame_rate = (
-            1 / config.frame_rate
-        )  # config.frame_rate holds the number of frames per second
-        for animation in animations:
-            if animation.run_time <= 0:
-                raise ValueError(
-                    f"{animation} has a run_time of <= 0 seconds which Manim cannot render. "
-                    "Please set the run_time to be positive."
-                )
-            elif animation.run_time < frame_rate:
-                logger.warning(
-                    f"Original run time of {animation} is shorter than current frame "
-                    f"rate (1 frame every {frame_rate:.2f} sec.) which cannot be rendered. "
-                    "Rendering with the shortest possible duration instead."
-                )
-                animation.run_time = frame_rate
-
-            if animation.run_time > max_run_time:
-                max_run_time = animation.run_time
-
-        return max_run_time
+        return max(animation.run_time for animation in animations)
 
     def play(
         self,

From 4da925c7f153d52b8341799b70c43b9e6310dd54 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= <francisco.manriquezn@usm.cl>
Date: Sat, 26 Oct 2024 23:40:22 -0300
Subject: [PATCH 2/3] Rewrite error and warning messages, and add validations
 to wait(), pause() and wait_until()

---
 manim/animation/animation.py             | 27 ++++++++++++++----------
 manim/animation/composition.py           |  6 +++---
 manim/scene/scene.py                     | 10 ++++++++-
 tests/module/animation/test_animation.py | 14 +++++-------
 4 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/manim/animation/animation.py b/manim/animation/animation.py
index c5e80f6e80..85d5092142 100644
--- a/manim/animation/animation.py
+++ b/manim/animation/animation.py
@@ -193,7 +193,7 @@ def begin(self) -> None:
         method.
 
         """
-        self.run_time = validate_animation_run_time(self.run_time, str(self))
+        self.run_time = validate_run_time(self.run_time, str(self))
         self.starting_mobject = self.create_starting_mobject()
         if self.suspend_mobject_updating:
             # All calls to self.mobject's internal updaters
@@ -568,22 +568,27 @@ def prepare_animation(
     raise TypeError(f"Object {anim} cannot be converted to an animation")
 
 
-def validate_animation_run_time(run_time: float, caller_name: str) -> float:
+def validate_run_time(
+    run_time: float, caller_name: str, parameter_name: str = "run_time"
+) -> float:
     if run_time <= 0:
         raise ValueError(
-            f"{caller_name} has a run_time of <= 0 seconds which Manim cannot render. "
-            "Please set the run_time to be positive."
+            f"{caller_name} has a {parameter_name} of {run_time:g} <= 0 "
+            f"seconds which Manim cannot render. Please set the "
+            f"{parameter_name} to a positive number."
         )
 
     # config.frame_rate holds the number of frames per second
-    frame_rate = 1 / config.frame_rate
-    if run_time < frame_rate:
+    fps = config.frame_rate
+    seconds_per_frame = 1 / fps
+    if run_time < seconds_per_frame:
         logger.warning(
-            f"Original run time of {caller_name} is shorter than current frame "
-            f"rate (1 frame every {frame_rate:.2f} sec.) which cannot be rendered. "
-            "Rendering with the shortest possible duration instead."
+            f"The original {parameter_name} of {caller_name}, {run_time:g} "
+            f"seconds, is too short for the current frame rate of {fps:g} "
+            f"FPS. Rendering with the shortest possible {parameter_name} of "
+            f"{seconds_per_frame:g} seconds instead."
         )
-        new_run_time = frame_rate
+        new_run_time = seconds_per_frame
     else:
         new_run_time = run_time
 
@@ -632,7 +637,7 @@ def __init__(
         self.mobject.shader_wrapper_list = []
 
     def begin(self) -> None:
-        self.run_time = validate_animation_run_time(self.run_time, str(self))
+        self.run_time = validate_run_time(self.run_time, str(self))
 
     def finish(self) -> None:
         pass
diff --git a/manim/animation/composition.py b/manim/animation/composition.py
index baa7958794..7ac20230de 100644
--- a/manim/animation/composition.py
+++ b/manim/animation/composition.py
@@ -12,7 +12,7 @@
 from manim.animation.animation import (
     Animation,
     prepare_animation,
-    validate_animation_run_time,
+    validate_run_time,
 )
 from manim.constants import RendererType
 from manim.mobject.mobject import Group, Mobject
@@ -91,7 +91,7 @@ def begin(self) -> None:
                 f"Trying to play {self} without animations, this is not supported. "
                 "Please add at least one subanimation."
             )
-        self.run_time = validate_animation_run_time(self.run_time, str(self))
+        self.run_time = validate_run_time(self.run_time, str(self))
         self.anim_group_time = 0.0
         if self.suspend_mobject_updating:
             self.group.suspend_updating()
@@ -239,7 +239,7 @@ def begin(self) -> None:
                 f"Trying to play {self} without animations, this is not supported. "
                 "Please add at least one subanimation."
             )
-        self.run_time = validate_animation_run_time(self.run_time, str(self))
+        self.run_time = validate_run_time(self.run_time, str(self))
         self.update_active_animation(0)
 
     def finish(self) -> None:
diff --git a/manim/scene/scene.py b/manim/scene/scene.py
index ed07fe453e..6673d1bbdf 100644
--- a/manim/scene/scene.py
+++ b/manim/scene/scene.py
@@ -34,7 +34,12 @@
 from watchdog.observers import Observer
 
 from manim._config import config, logger
-from manim.animation.animation import Animation, Wait, prepare_animation
+from manim.animation.animation import (
+    Animation,
+    Wait,
+    prepare_animation,
+    validate_run_time,
+)
 from manim.camera.camera import Camera
 from manim.constants import *
 from manim.gui.gui import configure_pygui
@@ -1125,6 +1130,7 @@ def wait(
         --------
         :class:`.Wait`, :meth:`.should_mobjects_update`
         """
+        duration = validate_run_time(duration, str(self) + ".wait()", "duration")
         self.play(
             Wait(
                 run_time=duration,
@@ -1148,6 +1154,7 @@ def pause(self, duration: float = DEFAULT_WAIT_TIME):
         --------
         :meth:`.wait`, :class:`.Wait`
         """
+        duration = validate_run_time(duration, str(self) + ".pause()", "duration")
         self.wait(duration=duration, frozen_frame=True)
 
     def wait_until(self, stop_condition: Callable[[], bool], max_time: float = 60):
@@ -1161,6 +1168,7 @@ def wait_until(self, stop_condition: Callable[[], bool], max_time: float = 60):
         max_time
             The maximum wait time in seconds.
         """
+        max_time = validate_run_time(max_time, str(self) + ".wait_until()", "max_time")
         self.wait(max_time, stop_condition=stop_condition)
 
     def compile_animation_data(
diff --git a/tests/module/animation/test_animation.py b/tests/module/animation/test_animation.py
index 448c0e76d6..fb2efc7853 100644
--- a/tests/module/animation/test_animation.py
+++ b/tests/module/animation/test_animation.py
@@ -11,24 +11,20 @@
 )
 def test_animation_forbidden_run_time(run_time):
     test_scene = Scene()
-    with pytest.raises(ValueError, match="Please set the run_time to be positive"):
+    with pytest.raises(
+        ValueError, match="Please set the run_time to a positive number."
+    ):
         test_scene.play(FadeIn(None, run_time=run_time))
 
 
 def test_animation_run_time_shorter_than_frame_rate(manim_caplog, config):
     test_scene = Scene()
     test_scene.play(FadeIn(None, run_time=1 / (config.frame_rate + 1)))
-    assert (
-        "Original run time of FadeIn(Mobject) is shorter than current frame rate"
-        in manim_caplog.text
-    )
+    assert "too short for the current frame rate" in manim_caplog.text
 
 
 @pytest.mark.parametrize("frozen_frame", [False, True])
 def test_wait_run_time_shorter_than_frame_rate(manim_caplog, frozen_frame):
     test_scene = Scene()
     test_scene.wait(1e-9, frozen_frame=frozen_frame)
-    assert (
-        "Original run time of Wait(Mobject) is shorter than current frame rate"
-        in manim_caplog.text
-    )
+    assert "too short for the current frame rate" in manim_caplog.text

From 9d8229ca8ebe01e6603dee63a2dc1c012164724d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Manr=C3=ADquez?= <francisco.manriquezn@usm.cl>
Date: Sun, 27 Oct 2024 09:56:19 -0300
Subject: [PATCH 3/3] Undo rewriting of imports

---
 manim/animation/animation.py             | 30 +++++++++++++++++++++++-
 manim/animation/composition.py           |  5 ++--
 manim/scene/scene.py                     | 28 ++++------------------
 tests/module/animation/test_animation.py | 14 ++++-------
 4 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/manim/animation/animation.py b/manim/animation/animation.py
index 8bdf236b3f..e090d99059 100644
--- a/manim/animation/animation.py
+++ b/manim/animation/animation.py
@@ -194,6 +194,7 @@ def begin(self) -> None:
         method.
 
         """
+        self.run_time = validate_run_time(self.run_time, str(self))
         self.starting_mobject = self.create_starting_mobject()
         if self.suspend_mobject_updating:
             # All calls to self.mobject's internal updaters
@@ -568,6 +569,33 @@ def prepare_animation(
     raise TypeError(f"Object {anim} cannot be converted to an animation")
 
 
+def validate_run_time(
+    run_time: float, caller_name: str, parameter_name: str = "run_time"
+) -> float:
+    if run_time <= 0:
+        raise ValueError(
+            f"{caller_name} has a {parameter_name} of {run_time:g} <= 0 "
+            f"seconds which Manim cannot render. Please set the "
+            f"{parameter_name} to a positive number."
+        )
+
+    # config.frame_rate holds the number of frames per second
+    fps = config.frame_rate
+    seconds_per_frame = 1 / fps
+    if run_time < seconds_per_frame:
+        logger.warning(
+            f"The original {parameter_name} of {caller_name}, {run_time:g} "
+            f"seconds, is too short for the current frame rate of {fps:g} "
+            f"FPS. Rendering with the shortest possible {parameter_name} of "
+            f"{seconds_per_frame:g} seconds instead."
+        )
+        new_run_time = seconds_per_frame
+    else:
+        new_run_time = run_time
+
+    return new_run_time
+
+
 class Wait(Animation):
     """A "no operation" animation.
 
@@ -610,7 +638,7 @@ def __init__(
         self.mobject.shader_wrapper_list = []
 
     def begin(self) -> None:
-        pass
+        self.run_time = validate_run_time(self.run_time, str(self))
 
     def finish(self) -> None:
         pass
diff --git a/manim/animation/composition.py b/manim/animation/composition.py
index c5a756502f..36e9e1d6e3 100644
--- a/manim/animation/composition.py
+++ b/manim/animation/composition.py
@@ -9,7 +9,7 @@
 import numpy as np
 
 from manim._config import config
-from manim.animation.animation import Animation, prepare_animation
+from manim.animation.animation import Animation, prepare_animation, validate_run_time
 from manim.constants import RendererType
 from manim.mobject.mobject import Group, Mobject
 from manim.mobject.opengl.opengl_mobject import OpenGLGroup
@@ -87,7 +87,7 @@ def begin(self) -> None:
                 f"Trying to play {self} without animations, this is not supported. "
                 "Please add at least one subanimation."
             )
-
+        self.run_time = validate_run_time(self.run_time, str(self))
         self.anim_group_time = 0.0
         if self.suspend_mobject_updating:
             self.group.suspend_updating()
@@ -235,6 +235,7 @@ def begin(self) -> None:
                 f"Trying to play {self} without animations, this is not supported. "
                 "Please add at least one subanimation."
             )
+        self.run_time = validate_run_time(self.run_time, str(self))
         self.update_active_animation(0)
 
     def finish(self) -> None:
diff --git a/manim/scene/scene.py b/manim/scene/scene.py
index 02c548cf7f..fe0f993413 100644
--- a/manim/scene/scene.py
+++ b/manim/scene/scene.py
@@ -37,7 +37,7 @@
 from manim.mobject.opengl.opengl_mobject import OpenGLPoint
 
 from .. import config, logger
-from ..animation.animation import Animation, Wait, prepare_animation
+from ..animation.animation import Animation, Wait, prepare_animation, validate_run_time
 from ..camera.camera import Camera
 from ..constants import *
 from ..gui.gui import configure_pygui
@@ -1030,28 +1030,7 @@ def get_run_time(self, animations: list[Animation]):
         float
             The total ``run_time`` of all of the animations in the list.
         """
-        max_run_time = 0
-        frame_rate = (
-            1 / config.frame_rate
-        )  # config.frame_rate holds the number of frames per second
-        for animation in animations:
-            if animation.run_time <= 0:
-                raise ValueError(
-                    f"{animation} has a run_time of <= 0 seconds which Manim cannot render. "
-                    "Please set the run_time to be positive."
-                )
-            elif animation.run_time < frame_rate:
-                logger.warning(
-                    f"Original run time of {animation} is shorter than current frame "
-                    f"rate (1 frame every {frame_rate:.2f} sec.) which cannot be rendered. "
-                    "Rendering with the shortest possible duration instead."
-                )
-                animation.run_time = frame_rate
-
-            if animation.run_time > max_run_time:
-                max_run_time = animation.run_time
-
-        return max_run_time
+        return max(animation.run_time for animation in animations)
 
     def play(
         self,
@@ -1147,6 +1126,7 @@ def wait(
         --------
         :class:`.Wait`, :meth:`.should_mobjects_update`
         """
+        duration = validate_run_time(duration, str(self) + ".wait()", "duration")
         self.play(
             Wait(
                 run_time=duration,
@@ -1170,6 +1150,7 @@ def pause(self, duration: float = DEFAULT_WAIT_TIME):
         --------
         :meth:`.wait`, :class:`.Wait`
         """
+        duration = validate_run_time(duration, str(self) + ".pause()", "duration")
         self.wait(duration=duration, frozen_frame=True)
 
     def wait_until(self, stop_condition: Callable[[], bool], max_time: float = 60):
@@ -1183,6 +1164,7 @@ def wait_until(self, stop_condition: Callable[[], bool], max_time: float = 60):
         max_time
             The maximum wait time in seconds.
         """
+        max_time = validate_run_time(max_time, str(self) + ".wait_until()", "max_time")
         self.wait(max_time, stop_condition=stop_condition)
 
     def compile_animation_data(
diff --git a/tests/module/animation/test_animation.py b/tests/module/animation/test_animation.py
index 448c0e76d6..fb2efc7853 100644
--- a/tests/module/animation/test_animation.py
+++ b/tests/module/animation/test_animation.py
@@ -11,24 +11,20 @@
 )
 def test_animation_forbidden_run_time(run_time):
     test_scene = Scene()
-    with pytest.raises(ValueError, match="Please set the run_time to be positive"):
+    with pytest.raises(
+        ValueError, match="Please set the run_time to a positive number."
+    ):
         test_scene.play(FadeIn(None, run_time=run_time))
 
 
 def test_animation_run_time_shorter_than_frame_rate(manim_caplog, config):
     test_scene = Scene()
     test_scene.play(FadeIn(None, run_time=1 / (config.frame_rate + 1)))
-    assert (
-        "Original run time of FadeIn(Mobject) is shorter than current frame rate"
-        in manim_caplog.text
-    )
+    assert "too short for the current frame rate" in manim_caplog.text
 
 
 @pytest.mark.parametrize("frozen_frame", [False, True])
 def test_wait_run_time_shorter_than_frame_rate(manim_caplog, frozen_frame):
     test_scene = Scene()
     test_scene.wait(1e-9, frozen_frame=frozen_frame)
-    assert (
-        "Original run time of Wait(Mobject) is shorter than current frame rate"
-        in manim_caplog.text
-    )
+    assert "too short for the current frame rate" in manim_caplog.text