Skip to content

Experimental: fix 11 tests #3979

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

Merged
merged 6 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions manim/animation/animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion manim/animation/composition.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions manim/mobject/opengl/opengl_vectorized_mobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion manim/mobject/text/text_mobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion manim/scene/scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -382,6 +382,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
Expand All @@ -393,6 +394,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(
Expand Down
3 changes: 2 additions & 1 deletion manim/scene/vector_space_scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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))

Expand Down
12 changes: 5 additions & 7 deletions tests/experimental/test_vmobject_init.py
Original file line number Diff line number Diff line change
@@ -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]
19 changes: 7 additions & 12 deletions tests/module/animation/test_animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably get frozen_frame working instead of removing it from the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frozen_frame was no longer among the parameters of the new Scene.wait(), so I assumed that it would no longer be needed.

I can put it there again, if it's necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that might be my fault actually - I think I might have wanted to replace it with wait_until?
I guess for now until wait_until works this is fine.

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
14 changes: 8 additions & 6 deletions tests/module/mobject/mobject/test_mobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
19 changes: 14 additions & 5 deletions tests/module/mobject/test_boolean_ops.py
Original file line number Diff line number Diff line change
@@ -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"),
Expand All @@ -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()
Expand 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)
10 changes: 5 additions & 5 deletions tests/test_linear_transformation_scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,22 @@

def test_ghost_vectors_len_and_types():
manager = Manager(LinearTransformationScene)
scene = manager.scene
scene: LinearTransformationScene = manager.scene
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's interesting that we need this annotation - I thought it should be able to infer this already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Honestly, I don't know why this happens.

scene.leave_ghost_vectors = True

# prepare vectors (they require a vmobject as their target)
# Prepare vectors. They require a VMobject as their target.
v1, v2 = Vector(RIGHT), Vector(RIGHT)
v1.target, v2.target = Vector(UP), Vector(UP)

# ghost_vector addition is in this method
# Ghost_vector addition is in this method:
scene.get_piece_movement((v1, v2))

ghosts = scene.get_ghost_vectors()
assert len(ghosts) == 1
# check if there are two vectors in the ghost vector VGroup
# Check if there are two vectors in the ghost vector VGroup.
assert len(ghosts[0]) == 2

# check types of ghost vectors
# Check types of ghost vectors.
assert isinstance(ghosts, VGroup)
assert isinstance(ghosts[0], VGroup)
assert all(isinstance(x, Vector) for x in ghosts[0])
2 changes: 1 addition & 1 deletion tests/test_scene_rendering/test_file_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def construct(self):
self.add_sound(file_path)
self.wait()

SceneWithMP3().render()
Manager(SceneWithMP3).render()
assert "click.mp3 to .wav" in manim_caplog.text


Expand Down
Loading