Experimental: fix 11 tests (#3979)

* Fix tests and verify Animation run time inside new run_time property setter

* test_boolean_ops

* Add run_time validation from main branch

* Fix 2 tests in tests/module/mobject/mobject/test_mobject.py

* Fix 2 tests in tests/module/mobject/text/test_text_mobject.py

* Manager in test_file_writer.py

Author: chopan050

manim/animation/animation.py

@@ -12,7 +12,7 @@
 
 from manim.mobject.opengl.opengl_mobject import OpenGLMobject
 
-from .. import logger
+from .. import config, logger
 from ..mobject import mobject
 from ..mobject.mobject import Mobject
 from ..mobject.opengl import opengl_mobject
@@ -209,6 +209,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
@@ -551,6 +552,33 @@ def prepare_animation(
         raise TypeError(f"Object {anim} cannot be converted to an animation") from None
 
 
+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.
 
@@ -590,7 +618,7 @@ def __init__(
         super().__init__(None, run_time=run_time, rate_func=rate_func, **kwargs)
 
     def begin(self) -> None:
-        pass
+        self.run_time = validate_run_time(self.run_time, str(self))
 
     def finish(self) -> None:
         pass

manim/animation/composition.py

@@ -8,7 +8,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
@@ -90,6 +90,7 @@ def begin(self) -> None:
             anim.begin()
             self.process_subanimation_buffer(anim.buffer)
 
+        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()
@@ -228,6 +229,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:

manim/mobject/opengl/opengl_vectorized_mobject.py

@@ -142,13 +142,13 @@ def get_grid(self, *args, **kwargs) -> OpenGLVGroup:  # type: ignore
     def __getitem__(self, value: int | slice) -> Self:  # type: ignore
         return super().__getitem__(value)  # type: ignore
 
-    def add(self, *vmobjects: OpenGLVMobject):  # type: ignore
+    def add(self, *vmobjects: OpenGLVMobject) -> Self:  # type: ignore
         if not all(isinstance(m, OpenGLVMobject) for m in vmobjects):
             raise Exception("All submobjects must be of type OpenGLVMobject")
-        super().add(*vmobjects)
+        return super().add(*vmobjects)
 
     # Colors
-    def init_colors(self):
+    def init_colors(self) -> Self:
         # self.set_fill(
         #     color=self.fill_color or self.color,
         #     opacity=self.fill_opacity,

manim/mobject/text/text_mobject.py

@@ -1236,7 +1236,7 @@ def __init__(
         else:
             self.line_spacing = self._font_size + self._font_size * self.line_spacing
 
-        color: ManimColor = ManimColor(color) if color else VMobject().color
+        color: ManimColor = ManimColor(color) if color else OpenGLVMobject().color
         file_name = self._text2svg(color)
 
         PangoUtils.remove_last_M(file_name)

manim/scene/scene.py

@@ -10,7 +10,7 @@
 from typing_extensions import assert_never
 
 from manim import config, logger
-from manim.animation.animation import prepare_animation
+from manim.animation.animation import prepare_animation, validate_run_time
 from manim.animation.scene_buffer import SceneBuffer, SceneOperation
 from manim.camera.camera import Camera
 from manim.constants import DEFAULT_WAIT_TIME
@@ -380,6 +380,7 @@ def wait(
         note: str | None = None,
         ignore_presenter_mode: bool = False,
     ):
+        duration = validate_run_time(duration, str(self) + ".wait()", "duration")
         self.manager._wait(duration, stop_condition=stop_condition)
         # if (
         #     self.presenter_mode
@@ -391,6 +392,7 @@ def wait(
         #     self.hold_loop()
 
     def wait_until(self, stop_condition: Callable[[], bool], max_time: float = 60):
+        max_time = validate_run_time(max_time, str(self) + ".wait_until()", "max_time")
         self.wait(max_time, stop_condition=stop_condition)
 
     def add_sound(

manim/scene/vector_space_scene.py

@@ -13,6 +13,7 @@
 from manim.mobject.geometry.polygram import Rectangle
 from manim.mobject.graphing.coordinate_systems import Axes, NumberPlane
 from manim.mobject.opengl.opengl_mobject import OpenGLMobject
+from manim.mobject.opengl.opengl_vectorized_mobject import OpenGLVMobject
 from manim.mobject.text.tex_mobject import MathTex, Tex
 from manim.utils.config_ops import update_dict_recursively
 
@@ -1002,7 +1003,7 @@ def get_piece_movement(self, pieces: list | tuple | np.ndarray):
         Animation
             The animation of the movement.
         """
-        v_pieces = [piece for piece in pieces if isinstance(piece, VMobject)]
+        v_pieces = [piece for piece in pieces if isinstance(piece, OpenGLVMobject)]
         start = VGroup(*v_pieces)
         target = VGroup(*(mob.target for mob in v_pieces))
 

tests/experimental/test_vmobject_init.py

@@ -1,16 +1,14 @@
 from manim import manim_colors as col
 from manim.mobject.opengl.opengl_vectorized_mobject import OpenGLVMobject
 
-VMobject = OpenGLVMobject
-
 
 def test_vmobject_init():
-    vm = VMobject(col.RED)
+    vm = OpenGLVMobject()
+    assert vm.fill_color == [col.WHITE]
+    assert vm.stroke_color == [col.WHITE]
+    vm = OpenGLVMobject(color=col.RED)
     assert vm.fill_color == [col.RED]
     assert vm.stroke_color == [col.RED]
-    vm = VMobject(col.GREEN, stroke_color=col.YELLOW)
+    vm = OpenGLVMobject(fill_color=col.GREEN, stroke_color=col.YELLOW)
     assert vm.fill_color == [col.GREEN]
     assert vm.stroke_color == [col.YELLOW]
-    vm = VMobject()
-    assert vm.fill_color == [col.WHITE]
-    assert vm.stroke_color == [col.WHITE]

tests/module/animation/test_animation.py

@@ -12,26 +12,21 @@
 def test_animation_forbidden_run_time(run_time):
     manager = Manager(Scene)
     test_scene = manager.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):
     manager = Manager(Scene)
     test_scene = manager.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):
+def test_wait_run_time_shorter_than_frame_rate(manim_caplog):
     manager = Manager(Scene)
     test_scene = manager.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
-    )
+    test_scene.wait(1e-9)
+    assert "too short for the current frame rate" in manim_caplog.text

tests/module/mobject/mobject/test_mobject.py

@@ -144,13 +144,15 @@ def test_mobject_dimensions_mobjects_with_no_points_are_at_origin():
     assert outer_group.width == 2
     assert outer_group.height == 3
 
-    # Adding a mobject with no points has a quirk of adding a "point"
-    # to [0, 0, 0] (the origin). This changes the size of the outer
-    # group because now the bottom left corner is at [-5, -6.5, 0]
-    # but the upper right corner is [0, 0, 0] instead of [-3, -3.5, 0]
+    # TODO: remove the following 8 lines?
+    # Originally, adding a mobject with no points had a quirk of adding a
+    # "point" to [0, 0, 0] (the origin). This changed the size of the outer
+    # group, because the bottom was corner is at [-5, -6.5, 0], but the
+    # upper right corner became [0, 0, 0] instead of [-3, -3.5, 0].
+    # However, this no longer happens.
     outer_group.add(VGroup())
-    assert outer_group.width == 5
-    assert outer_group.height == 6.5
+    assert outer_group.width == 2
+    assert outer_group.height == 3
 
 
 def test_mobject_dimensions_has_points_and_children():

tests/module/mobject/test_boolean_ops.py

@@ -1,11 +1,17 @@
 from __future__ import annotations
 
+from typing import TYPE_CHECKING
+
 import numpy as np
 import pytest
 
 from manim import Circle, Square
 from manim.mobject.geometry.boolean_ops import _BooleanOps
 
+if TYPE_CHECKING:
+    from manim.mobject.types.vectorized_mobject import VMobject
+    from manim.typing import Point2D_Array, Point3D_Array
+
 
 @pytest.mark.parametrize(
     ("test_input", "expected"),
@@ -25,15 +31,17 @@
         ),
     ],
 )
-def test_convert_2d_to_3d_array(test_input, expected):
+def test_convert_2d_to_3d_array(
+    test_input: Point2D_Array, expected: Point3D_Array
+) -> None:
     a = _BooleanOps()
     result = a._convert_2d_to_3d_array(test_input)
     assert len(result) == len(expected)
     for i in range(len(result)):
         assert (result[i] == expected[i]).all()
 
 
-def test_convert_2d_to_3d_array_zdim():
+def test_convert_2d_to_3d_array_zdim() -> None:
     a = _BooleanOps()
     result = a._convert_2d_to_3d_array([(1.0, 2.0)], z_dim=1.0)
     assert (result[0] == np.array([1.0, 2.0, 1.0])).all()
@@ -48,11 +56,12 @@ def test_convert_2d_to_3d_array_zdim():
         Circle(radius=3),
     ],
 )
-def test_vmobject_to_skia_path_and_inverse(test_input):
+def test_vmobject_to_skia_path_and_inverse(test_input: VMobject) -> None:
     a = _BooleanOps()
     path = a._convert_vmobject_to_skia_path(test_input)
     assert len(list(path.segments)) > 1
 
     new_vmobject = a._convert_skia_path_to_vmobject(path)
-    # for some reason there is an extra 4 points in new vmobject than original
-    np.testing.assert_allclose(new_vmobject.points[:-4], test_input.points)
+    # For some reason, there are 3 more points in the new VMobject than in the
+    # original input.
+    np.testing.assert_allclose(new_vmobject.points[:-3], test_input.points)