ronantakizawa/codereview-bench huggingface dataset:
Repository: ManimCommunity/manim
File: tests/test_auto_zoom.py (Python)

Code before:
from manim import *


def test_zoom():

    s1 = Square()
    s1.set_x(-10)
    s2 = Square()
    s2.set_x(10)

    with tempconfig({"dry_run": True}):
        scene = MovingCameraScene()
        scene.add(s1, s2)
        scene.play(scene.camera.auto_zoom([s1, s2]))

    assert (
        scene.camera.frame_width
        == abs(
            s1.get_left()[0] - s2.get_right()[0],
        )
        and scene.camera.frame.get_center()[0]
        == (abs(s1.get_center()[0] + s2.get_center()[0]) / 2)
    )


Review comment (refactor):
```suggestion
    with tempconfig({"dry_run": True, "quality": "low"}):
```

Code after:
from manim import *


def test_zoom():

    s1 = Square()
    s1.set_x(-10)
    s2 = Square()
    s2.set_x(10)

    with tempconfig({"dry_run": True, "quality": "low_quality"}):
        scene = MovingCameraScene()
        scene.add(s1, s2)
        scene.play(scene.camera.auto_zoom([s1, s2]))

    assert (
        scene.camera.frame_width
        == abs(
            s1.get_left()[0] - s2.get_right()[0],
        )
        and scene.camera.frame.get_center()[0]
        == (abs(s1.get_center()[0] + s2.get_center()[0]) / 2)
    )
Repository: ManimCommunity/manim
File: tests/test_auto_zoom.py (Python)

Code before:
from manim import *


def test_zoom():

    s1 = Square()
    s1.set_x(-10)
    s2 = Square()
    s2.set_x(10)

    with tempconfig({"dry_run": True, "quality": "low"}):
        scene = MovingCameraScene()
        scene.add(s1, s2)
        scene.play(scene.camera.auto_zoom([s1, s2]))

    assert (
        scene.camera.frame_width
        == abs(
            s1.get_left()[0] - s2.get_right()[0],
        )
        and scene.camera.frame.get_center()[0]
        == (abs(s1.get_center()[0] + s2.get_center()[0]) / 2)
    )


Review comment (refactor):
```suggestion
    with tempconfig({"dry_run": True, "quality": "low_quality"}):
```

Code after:
from manim import *


def test_zoom():

    s1 = Square()
    s1.set_x(-10)
    s2 = Square()
    s2.set_x(10)

    with tempconfig({"dry_run": True, "quality": "low_quality"}):
        scene = MovingCameraScene()
        scene.add(s1, s2)
        scene.play(scene.camera.auto_zoom([s1, s2]))

    assert (
        scene.camera.frame_width
        == abs(
            s1.get_left()[0] - s2.get_right()[0],
        )
        and scene.camera.frame.get_center()[0]
        == (abs(s1.get_center()[0] + s2.get_center()[0]) / 2)
    )
Repository: ManimCommunity/manim
File: tests/test_scene_rendering/opengl/test_cli_flags_opengl.py (Python)

Code before:
    command = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "-ql",
        "--write_to_movie",
        "--media_dir",
        str(tmp_path),
        simple_scenes_path,
        scene_name,
    ]
    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 = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "--write_to_movie",
        "-n 3,6",
        "--media_dir",
        str(tmp_path),
        simple_scenes_path,
        scene_name,
    ]
    _, err, exit_code = capture(command)
    assert exit_code == 0, err


@pytest.mark.slow
def test_s_flag_no_animations(tmp_path, manim_cfg_file, simple_scenes_path):
    scene_name = "NoAnimations"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "-ql",

Review comment (suggestion):
Could you rename this to something like "video_config". Reason is that `config` is used as manim's global config, not really an issue atm but in future if someone imports it in here it could get overwritten and also avoids confusion with the global config.

Code after:
        (1920, 1080),
        (2560, 1440),
        (3840, 2160),
        (640, 480),
        (800, 600),
    ]

    separators = [";", ",", "-"]

    for (width, height), separator in itertools.product(resolutions, separators):
        command = [
            sys.executable,
            "-m",
            "manim",
            "--media_dir",
            str(tmp_path),
            "--resolution",
            f"{width}{separator}{height}",
            str(simple_scenes_path),
            scene_name,
        ]

        _, err, exit_code = capture(command)
        assert exit_code == 0, err

        path = (
            tmp_path / "videos" / "simple_scenes" / f"{height}p60" / f"{scene_name}.mp4"
        )
        meta = get_video_metadata(path)
        assert (width, height) == (meta["width"], meta["height"])


@pytest.mark.slow
@video_comparison(
    "SquareToCircleWithlFlag.json",
    "videos/simple_scenes/480p15/SquareToCircle.mp4",
)
def test_basic_scene_l_flag(tmp_path, manim_cfg_file, simple_scenes_path):
    scene_name = "SquareToCircle"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "-ql",
        "--write_to_movie",
        "--media_dir",
        str(tmp_path),
        simple_scenes_path,
        scene_name,
Repository: ManimCommunity/manim
File: tests/test_scene_rendering/opengl/test_cli_flags_opengl.py (Python)

Code before:


@pytest.mark.slow
@video_comparison(
    "SquareToCircleWithDefaultValues.json",
    "videos/simple_scenes/1080p60/SquareToCircle.mp4",
)
def test_basic_scene_with_default_values(tmp_path, manim_cfg_file, simple_scenes_path):
    scene_name = "SquareToCircle"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "--write_to_movie",
        "--media_dir",
        str(tmp_path),
        simple_scenes_path,
        scene_name,
    ]
    out, err, exit_code = capture(command)
    assert exit_code == 0, err


@pytest.mark.slow
@video_comparison(
    "SquareToCircleWithlFlag.json",
    "videos/simple_scenes/480p15/SquareToCircle.mp4",
)
def test_basic_scene_l_flag(tmp_path, manim_cfg_file, simple_scenes_path):
    scene_name = "SquareToCircle"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "-ql",
        "--write_to_movie",
        "--media_dir",
        str(tmp_path),
        simple_scenes_path,
        scene_name,
    ]
    out, err, exit_code = capture(command)
    assert exit_code == 0, err


@pytest.mark.slow
@video_comparison(

Review comment (performance):
Due to the nature of this test it will be slow and I know it is marked as slow, but I think we can still make it a bit faster as it took a really long time to run locally. If you add a scene into `simple_scenes.py` such as:

```py
class ShortAnimationScene(Scene):
    def construct(self):
        dot = Dot()
        self.add(dot)
        self.wait(duration=0.1)
```

And use this instead of `SquareToCircle` as it still tests what you want but will be a good bit faster.

Code after:
from tests.utils.video_tester import video_comparison


@pytest.mark.slow
@video_comparison(
    "SquareToCircleWithDefaultValues.json",
    "videos/simple_scenes/1080p60/SquareToCircle.mp4",
)
def test_basic_scene_with_default_values(tmp_path, manim_cfg_file, simple_scenes_path):
    scene_name = "SquareToCircle"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--renderer",
        "opengl",
        "--write_to_movie",
        "--media_dir",
        str(tmp_path),
        simple_scenes_path,
        scene_name,
    ]
    out, err, exit_code = capture(command)
    assert exit_code == 0, err


@pytest.mark.slow()
def test_resolution_flag(tmp_path, manim_cfg_file, simple_scenes_path):
    scene_name = "NoAnimations"
    resolutions = [
        (720, 480),
        (1280, 720),
        (1920, 1080),
        (2560, 1440),
        (3840, 2160),
        (640, 480),
        (800, 600),
    ]

    separators = [";", ",", "-"]

    for (width, height), separator in itertools.product(resolutions, separators):
        command = [
            sys.executable,
            "-m",
            "manim",
            "--media_dir",
            str(tmp_path),
            "--resolution",
            f"{width}{separator}{height}",
            str(simple_scenes_path),
Repository: ManimCommunity/manim
File: manim/camera/camera.py (Python)

Code before:

        Parameters
        ----------
        mobject : Mobject
            The mobject for which the checking needs to be done.

        Returns
        -------
        bool
            True if in frame, False otherwise.
        """
        fc = self.frame_center
        fh = self.frame_height
        fw = self.frame_width
        return not reduce(
            op.or_,
            [
                mobject.get_right()[0] < fc[0] - fw,
                mobject.get_bottom()[1] > fc[1] + fh,
                mobject.get_left()[0] > fc[0] + fw,
                mobject.get_top()[1] < fc[1] - fh,
            ],
        )

    def capture_mobject(self, mobject, **kwargs):
        """Capture mobjects by printing them on :attr:`pixel_array`.

        This is a single-mobject version of :meth:`capture_mobjects`.

        Parameters
        ----------
        mobject : :class:`~.Mobject`
            Mobject to capture.

        kwargs : Any
            Keyword arguments to be passed to :meth:`get_mobjects_to_display`.

        """
        return self.capture_mobjects([mobject], **kwargs)

    def capture_mobjects(self, mobjects, **kwargs):
        """Capture mobjects by printing them on :attr:`pixel_array`.

        This is the essential function that converts the contents of a Scene
        into an array, which is then converted to an image or video.

        Parameters
        ----------
        mobjects : :class:`~.Mobject`
            Mobjects to capture.


Review comment (suggestion):
```suggestion
        """Capture mobject by storing it to :attr:`pixel_array`.
```

Minor improvement of wording.

Code after:

        Parameters
        ----------
        mobject : Mobject
            The mobject for which the checking needs to be done.

        Returns
        -------
        bool
            True if in frame, False otherwise.
        """
        fc = self.frame_center
        fh = self.frame_height
        fw = self.frame_width
        return not reduce(
            op.or_,
            [
                mobject.get_right()[0] < fc[0] - fw,
                mobject.get_bottom()[1] > fc[1] + fh,
                mobject.get_left()[0] > fc[0] + fw,
                mobject.get_top()[1] < fc[1] - fh,
            ],
        )

    def capture_mobject(self, mobject: Mobject, **kwargs: Any):
        """Capture mobjects by storing it in :attr:`pixel_array`.

        This is a single-mobject version of :meth:`capture_mobjects`.

        Parameters
        ----------
        mobject
            Mobject to capture.

        kwargs
            Keyword arguments to be passed to :meth:`get_mobjects_to_display`.

        """
        return self.capture_mobjects([mobject], **kwargs)

    def capture_mobjects(self, mobjects, **kwargs):
        """Capture mobjects by printing them on :attr:`pixel_array`.

        This is the essential function that converts the contents of a Scene
        into an array, which is then converted to an image or video.

        Parameters
        ----------
        mobjects : :class:`~.Mobject`
            Mobjects to capture.
Repository: ManimCommunity/manim
File: manim/utils/config_ops.py (Python)

Code before:
            result[key] = value
    return result


def update_dict_recursively(current_dict, *others):
    updated_dict = merge_dicts_recursively(current_dict, *others)
    current_dict.update(updated_dict)


# Occasionally convenient in order to write dict.x instead of more laborious
# (and less in keeping with all other attr accesses) dict["x"]


class DictAsObject:
    def __init__(self, dictin):
        self.__dict__ = dictin


class _Data:
    """Descriptor that allows _Data variables to be grouped and accessed from self.data["attr"] via self.attr.
    self.data attributes must be arrays.
    """

    def __set_name__(self, obj, name):
        self.name = name

    def __get__(self, obj, owner):
        return obj.__dict__["data"][self.name]

    def __set__(self, obj, array: np.ndarray):
        obj.__dict__["data"][self.name] = array


class _Uniforms:
    """Descriptor that allows _Uniforms variables to be grouped from self.uniforms["attr"] via self.attr.
    self.uniforms attributes must be floats.
    """

    def __set_name__(self, obj, name):
        self.name = name

    def __get__(self, obj, owner):
        return obj.__dict__["uniforms"][self.name]

    def __set__(self, obj, num: float):
        obj.__dict__["uniforms"][self.name] = num


Review comment (performance):
Checking if an attribute is in `data` might prove to be too expensive and cause a performance dropoff, since `data` is called a lot of times (haven't tested, just speculation). Is there a way to get around doing this?

Code after:
            result[key] = value
    return result


def update_dict_recursively(current_dict, *others):
    updated_dict = merge_dicts_recursively(current_dict, *others)
    current_dict.update(updated_dict)


# Occasionally convenient in order to write dict.x instead of more laborious
# (and less in keeping with all other attr accesses) dict["x"]


class DictAsObject:
    def __init__(self, dictin):
        self.__dict__ = dictin


class _Data:
    """Descriptor that allows _Data variables to be grouped and accessed from self.data["attr"] via self.attr.
    self.data attributes must be arrays.
    """

    def __set_name__(self, obj, name):
        self.name = name

    def __get__(self, obj, owner):
        return obj.data[self.name]

    def __set__(self, obj, array: np.ndarray):
        obj.data[self.name] = array


class _Uniforms:
    """Descriptor that allows _Uniforms variables to be grouped from self.uniforms["attr"] via self.attr.
    self.uniforms attributes must be floats.
    """

    def __set_name__(self, obj, name):
        self.name = name

    def __get__(self, obj, owner):
        return obj.__dict__["uniforms"][self.name]

    def __set__(self, obj, num: float):
        obj.__dict__["uniforms"][self.name] = num
Repository: ManimCommunity/manim
File: manim/animation/transform.py (Python)

Code before:
class ShrinkToCenter(ScaleInPlace):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject, 0, **kwargs)


class Restore(ApplyMethod):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject.restore, **kwargs)


class ApplyFunction(Transform):
    def __init__(self, function: types.MethodType, mobject: Mobject, **kwargs) -> None:
        self.function = function
        super().__init__(mobject, **kwargs)

    def create_target(self) -> Any:
        target = self.function(self.mobject.copy())
        if not isinstance(target, (Mobject, OpenGLMobject)):
            raise TypeError(
                "Functions passed to ApplyFunction must return object of type Mobject",
            )
        return target


class ApplyMatrix(ApplyPointwiseFunction):
    """Applies a matrix transform to an mobject

    Parameters
    ----------
    matrix
        The matrix transform
    mobject
        The :class:`~.Mobject`.
    about_point
        The origin point for the transform. Defaults to ORIGIN
    kwargs
        Further keyword arguments that are passed to :class:`ApplyPointwiseFunction`.
    """

    def __init__(
        self,
        matrix: np.ndarray,
        mobject: Mobject,
        about_point: np.ndarray = ORIGIN,
        **kwargs,
    ) -> None:
        matrix = self.initialize_matrix(matrix)

        def func(p):
            return np.dot(p - about_point, matrix.T) + about_point


Review comment (suggestion):
```suggestion
    """Applies a matrix transform to an mobject.
```

Code after:


class ScaleInPlace(ApplyMethod):
    def __init__(self, mobject: Mobject, scale_factor: float, **kwargs) -> None:
        super().__init__(mobject.scale, scale_factor, **kwargs)


class ShrinkToCenter(ScaleInPlace):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject, 0, **kwargs)


class Restore(ApplyMethod):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject.restore, **kwargs)


class ApplyFunction(Transform):
    def __init__(self, function: types.MethodType, mobject: Mobject, **kwargs) -> None:
        self.function = function
        super().__init__(mobject, **kwargs)

    def create_target(self) -> Any:
        target = self.function(self.mobject.copy())
        if not isinstance(target, (Mobject, OpenGLMobject)):
            raise TypeError(
                "Functions passed to ApplyFunction must return object of type Mobject",
            )
        return target


class ApplyMatrix(ApplyPointwiseFunction):
    """Applies a matrix transform to an mobject.

    Parameters
    ----------
    matrix
        The transformation matrix.
    mobject
        The :class:`~.Mobject`.
    about_point
        The origin point for the transform. Defaults to ``ORIGIN``.
    kwargs
        Further keyword arguments that are passed to :class:`ApplyPointwiseFunction`.
    """

    def __init__(
        self,
        matrix: np.ndarray,
        mobject: Mobject,
        about_point: np.ndarray = ORIGIN,
Repository: ManimCommunity/manim
File: manim/animation/transform.py (Python)

Code before:
class Restore(ApplyMethod):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject.restore, **kwargs)


class ApplyFunction(Transform):
    def __init__(self, function: types.MethodType, mobject: Mobject, **kwargs) -> None:
        self.function = function
        super().__init__(mobject, **kwargs)

    def create_target(self) -> Any:
        target = self.function(self.mobject.copy())
        if not isinstance(target, (Mobject, OpenGLMobject)):
            raise TypeError(
                "Functions passed to ApplyFunction must return object of type Mobject",
            )
        return target


class ApplyMatrix(ApplyPointwiseFunction):
    """Applies a matrix transform to an mobject

    Parameters
    ----------
    matrix
        The matrix transform
    mobject
        The :class:`~.Mobject`.
    about_point
        The origin point for the transform. Defaults to ORIGIN
    kwargs
        Further keyword arguments that are passed to :class:`ApplyPointwiseFunction`.
    """

    def __init__(
        self,
        matrix: np.ndarray,
        mobject: Mobject,
        about_point: np.ndarray = ORIGIN,
        **kwargs,
    ) -> None:
        matrix = self.initialize_matrix(matrix)

        def func(p):
            return np.dot(p - about_point, matrix.T) + about_point

        super().__init__(func, mobject, **kwargs)

    def initialize_matrix(self, matrix: np.ndarray) -> np.ndarray:
        matrix = np.array(matrix)
        if matrix.shape == (2, 2):

Review comment (suggestion):
```suggestion
        The transformation matrix.
```

Code after:


class ShrinkToCenter(ScaleInPlace):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject, 0, **kwargs)


class Restore(ApplyMethod):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject.restore, **kwargs)


class ApplyFunction(Transform):
    def __init__(self, function: types.MethodType, mobject: Mobject, **kwargs) -> None:
        self.function = function
        super().__init__(mobject, **kwargs)

    def create_target(self) -> Any:
        target = self.function(self.mobject.copy())
        if not isinstance(target, (Mobject, OpenGLMobject)):
            raise TypeError(
                "Functions passed to ApplyFunction must return object of type Mobject",
            )
        return target


class ApplyMatrix(ApplyPointwiseFunction):
    """Applies a matrix transform to an mobject.

    Parameters
    ----------
    matrix
        The transformation matrix.
    mobject
        The :class:`~.Mobject`.
    about_point
        The origin point for the transform. Defaults to ``ORIGIN``.
    kwargs
        Further keyword arguments that are passed to :class:`ApplyPointwiseFunction`.
    """

    def __init__(
        self,
        matrix: np.ndarray,
        mobject: Mobject,
        about_point: np.ndarray = ORIGIN,
        **kwargs,
    ) -> None:
        matrix = self.initialize_matrix(matrix)

        def func(p):
Repository: ManimCommunity/manim
File: manim/animation/transform.py (Python)

Code before:

class ApplyFunction(Transform):
    def __init__(self, function: types.MethodType, mobject: Mobject, **kwargs) -> None:
        self.function = function
        super().__init__(mobject, **kwargs)

    def create_target(self) -> Any:
        target = self.function(self.mobject.copy())
        if not isinstance(target, (Mobject, OpenGLMobject)):
            raise TypeError(
                "Functions passed to ApplyFunction must return object of type Mobject",
            )
        return target


class ApplyMatrix(ApplyPointwiseFunction):
    """Applies a matrix transform to an mobject

    Parameters
    ----------
    matrix
        The matrix transform
    mobject
        The :class:`~.Mobject`.
    about_point
        The origin point for the transform. Defaults to ORIGIN
    kwargs
        Further keyword arguments that are passed to :class:`ApplyPointwiseFunction`.
    """

    def __init__(
        self,
        matrix: np.ndarray,
        mobject: Mobject,
        about_point: np.ndarray = ORIGIN,
        **kwargs,
    ) -> None:
        matrix = self.initialize_matrix(matrix)

        def func(p):
            return np.dot(p - about_point, matrix.T) + about_point

        super().__init__(func, mobject, **kwargs)

    def initialize_matrix(self, matrix: np.ndarray) -> np.ndarray:
        matrix = np.array(matrix)
        if matrix.shape == (2, 2):
            new_matrix = np.identity(3)
            new_matrix[:2, :2] = matrix
            matrix = new_matrix
        elif matrix.shape != (3, 3):

Review comment (suggestion):
```suggestion
        The origin point for the transform. Defaults to ``ORIGIN``.
```

Code after:
        super().__init__(mobject, 0, **kwargs)


class Restore(ApplyMethod):
    def __init__(self, mobject: Mobject, **kwargs) -> None:
        super().__init__(mobject.restore, **kwargs)


class ApplyFunction(Transform):
    def __init__(self, function: types.MethodType, mobject: Mobject, **kwargs) -> None:
        self.function = function
        super().__init__(mobject, **kwargs)

    def create_target(self) -> Any:
        target = self.function(self.mobject.copy())
        if not isinstance(target, (Mobject, OpenGLMobject)):
            raise TypeError(
                "Functions passed to ApplyFunction must return object of type Mobject",
            )
        return target


class ApplyMatrix(ApplyPointwiseFunction):
    """Applies a matrix transform to an mobject.

    Parameters
    ----------
    matrix
        The transformation matrix.
    mobject
        The :class:`~.Mobject`.
    about_point
        The origin point for the transform. Defaults to ``ORIGIN``.
    kwargs
        Further keyword arguments that are passed to :class:`ApplyPointwiseFunction`.
    """

    def __init__(
        self,
        matrix: np.ndarray,
        mobject: Mobject,
        about_point: np.ndarray = ORIGIN,
        **kwargs,
    ) -> None:
        matrix = self.initialize_matrix(matrix)

        def func(p):
            return np.dot(p - about_point, matrix.T) + about_point

        super().__init__(func, mobject, **kwargs)