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

Code before:
    "MAROON_D",
    "MAROON_E",
    "MAROON",
    "PURPLE_A",
    "PURPLE_B",
    "PURPLE_C",
    "PURPLE_D",
    "PURPLE_E",
    "PURPLE",
    "PINK",
    "LIGHT_PINK",
    "ORANGE",
    "LIGHT_BROWN",
    "DARK_BROWN",
    "GRAY_BROWN",
    "GREY_BROWN",
]


def color_to_rgb(color: Union[Color, str]) -> np.ndarray:
    if isinstance(color, str):
        return hex_to_rgb(color)
    elif isinstance(color, Color):
        return np.array(color.get_rgb())
    else:
        raise ValueError("Invalid color type " + str(color))


def color_to_rgba(color: Union[Color, str], alpha: float = 1) -> np.ndarray:
    return np.array([*color_to_rgb(color), alpha])


def rgb_to_color(rgb: Iterable[float]) -> Color:
    return Color(rgb=rgb)


def rgba_to_color(rgba: Iterable[float]) -> Color:
    return rgb_to_color(rgba[:3])


def rgb_to_hex(rgb: Iterable[float]) -> str:
    return "#" + "".join("%02x" % int(255 * x) for x in rgb)


def hex_to_rgb(hex_code: str) -> np.ndarray:
    hex_part = hex_code[1:]
    if len(hex_part) == 3:
        hex_part = "".join([2 * c for c in hex_part])
    return np.array([int(hex_part[i : i + 2], 16) / 255 for i in range(0, 6, 2)])



Review comment (suggestion):
```suggestion
        raise ValueError("Invalid color type: " + str(color))
```

Code after:
    "MAROON_D",
    "MAROON_E",
    "MAROON",
    "PURPLE_A",
    "PURPLE_B",
    "PURPLE_C",
    "PURPLE_D",
    "PURPLE_E",
    "PURPLE",
    "PINK",
    "LIGHT_PINK",
    "ORANGE",
    "LIGHT_BROWN",
    "DARK_BROWN",
    "GRAY_BROWN",
    "GREY_BROWN",
]


def color_to_rgb(color: Union[Color, str]) -> np.ndarray:
    if isinstance(color, str):
        return hex_to_rgb(color)
    elif isinstance(color, Color):
        return np.array(color.get_rgb())
    else:
        raise ValueError("Invalid color type: " + str(color))


def color_to_rgba(color: Union[Color, str], alpha: float = 1) -> np.ndarray:
    return np.array([*color_to_rgb(color), alpha])


def rgb_to_color(rgb: Iterable[float]) -> Color:
    return Color(rgb=rgb)


def rgba_to_color(rgba: Iterable[float]) -> Color:
    return rgb_to_color(rgba[:3])


def rgb_to_hex(rgb: Iterable[float]) -> str:
    return "#" + "".join("%02x" % int(255 * x) for x in rgb)


def hex_to_rgb(hex_code: str) -> np.ndarray:
    hex_part = hex_code[1:]
    if len(hex_part) == 3:
        hex_part = "".join([2 * c for c in hex_part])
    return np.array([int(hex_part[i : i + 2], 16) / 255 for i in range(0, 6, 2)])
Repository: ManimCommunity/manim
File: manim/mobject/types/dot_cloud.py (Python)

Code before:
from .opengl_point_cloud_mobject import OpenGLPMObject


class DotCloud(OpenGLPMObject):
    def __init__(
        self, color=YELLOW, stroke_width=2.0, radius=2.0, density=10, **kwargs
    ):
        self.radius = radius
        self.epsilon = 1.0 / density
        super().__init__(
            stroke_width=stroke_width, density=density, color=color, **kwargs
        )

    def init_points(self):
        self.reset_points()
        self.data["points"] = np.array(self.generate_points(), dtype=np.float32)

    def generate_points(self):
        return [
            r * (np.cos(theta) * RIGHT + np.sin(theta) * UP)
            for r in np.arange(self.epsilon, self.radius, self.epsilon)
            # Num is equal to int(stop - start)/ (step + 1) reformulated.
            for theta in np.linspace(
                0, 2 * np.pi, num=int(2 * np.pi * (r + self.epsilon) / self.epsilon)
            )
        ]

    def make_3d(self, gloss=0.5, shadow=0.2):
        self.set_gloss(gloss)
        self.set_shadow(shadow)
        self.apply_depth_test()
        return self


class TrueDot(DotCloud):
    def __init__(self, center=ORIGIN, stroke_width=2.0, **kwargs):
        self.radius = stroke_width
        super().__init__(points=[center], stroke_width=stroke_width, **kwargs)


Review comment (suggestion):
since this is an OpenGL specific method, just calling it `init_points` should save the hassle of defining another `init_points`.

Code after:
from .opengl_point_cloud_mobject import OpenGLPMobject


class DotCloud(OpenGLPMobject):
    def __init__(
        self, color=YELLOW, stroke_width=2.0, radius=2.0, density=10, **kwargs
    ):
        self.radius = radius
        self.epsilon = 1.0 / density
        super().__init__(
            stroke_width=stroke_width, density=density, color=color, **kwargs
        )

    def init_points(self):
        self.points = np.array(
            [
                r * (np.cos(theta) * RIGHT + np.sin(theta) * UP)
                for r in np.arange(self.epsilon, self.radius, self.epsilon)
                # Num is equal to int(stop - start)/ (step + 1) reformulated.
                for theta in np.linspace(
                    0, 2 * np.pi, num=int(2 * np.pi * (r + self.epsilon) / self.epsilon)
                )
            ],
            dtype=np.float32,
        )

    def make_3d(self, gloss=0.5, shadow=0.2):
        self.set_gloss(gloss)
        self.set_shadow(shadow)
        self.apply_depth_test()
        return self


class TrueDot(DotCloud):
    def __init__(self, center=ORIGIN, stroke_width=2.0, **kwargs):
        self.radius = stroke_width
        super().__init__(points=[center], stroke_width=stroke_width, **kwargs)
Repository: ManimCommunity/manim
File: manim/mobject/geometry.py (Python)

Code before:
        if scale_tips:
            super().scale(factor, **kwargs)
            self.set_stroke_width_from_length()
            return self

        has_tip = self.has_tip()
        has_start_tip = self.has_start_tip()
        if has_tip or has_start_tip:
            old_tips = self.pop_tips()

        super().scale(factor, **kwargs)
        self.set_stroke_width_from_length()

        if has_tip:
            self.add_tip(tip=old_tips[0])
        if has_start_tip:
            self.add_tip(tip=old_tips[1], at_start=True)
        return self

    def get_normal_vector(self) -> np.ndarray:
        """Returns the normal of a vector.

        Examples
        --------
        ::

            >>> np.round(Arrow().get_normal_vector()) + 0. # add 0. to avoid negative 0 in output
            array([ 0.,  0., -1.])
        """

        p0, p1, p2 = self.tip.get_start_anchors()[:3]
        return normalize(np.cross(p2 - p1, p1 - p0))

    def reset_normal_vector(self):
        """Resets the normal of a vector"""
        self.normal_vector = self.get_normal_vector()
        return self

    def get_default_tip_length(self) -> float:
        """Returns the default tip_length of the arrow.

        Examples
        --------

        ::

            >>> Arrow().get_default_tip_length()
            0.35
        """

        max_ratio = self.max_tip_length_to_length_ratio

Review comment (suggestion):
```suggestion
            >>> np.around(Arrow().get_normal_vector(), decimals=4) + 0. # add 0. to avoid negative 0 in output
```

Code after:
            super().scale(factor, **kwargs)
            self.set_stroke_width_from_length()
            return self

        has_tip = self.has_tip()
        has_start_tip = self.has_start_tip()
        if has_tip or has_start_tip:
            old_tips = self.pop_tips()

        super().scale(factor, **kwargs)
        self.set_stroke_width_from_length()

        if has_tip:
            self.add_tip(tip=old_tips[0])
        if has_start_tip:
            self.add_tip(tip=old_tips[1], at_start=True)
        return self

    def get_normal_vector(self) -> np.ndarray:
        """Returns the normal of a vector.

        Examples
        --------
        ::

            >>> np.round(Arrow().get_normal_vector()) + 0. # add 0. to avoid negative 0 in output
            array([ 0.,  0., -1.])
        """

        p0, p1, p2 = self.tip.get_start_anchors()[:3]
        return normalize(np.cross(p2 - p1, p1 - p0))

    def reset_normal_vector(self):
        """Resets the normal of a vector"""
        self.normal_vector = self.get_normal_vector()
        return self

    def get_default_tip_length(self) -> float:
        """Returns the default tip_length of the arrow.

        Examples
        --------

        ::

            >>> Arrow().get_default_tip_length()
            0.35
        """

        max_ratio = self.max_tip_length_to_length_ratio
        return min(self.tip_length, max_ratio * self.get_length())
Repository: ManimCommunity/manim
File: manim/cli/default_group.py (Python)

Code before:
"""DefaultGroup allows a subcommand to act as the main command

In particular, this class is what allows `manim` to act as `manim render`.
"""
import cloup

from .. import logger

__all__ = ["DefaultGroup"]


class DefaultGroup(cloup.Group):
    """Invokes a subcommand marked with `default=True` if any subcommand not
    chosen.

    :param default_if_no_args: resolves to the default command if no arguments
                               passed.

    """

    def __init__(self, *args, **kwargs):
        # To resolve as the default command.
        if not kwargs.get("ignore_unknown_options", True):
            raise ValueError("Default group accepts unknown options")
        self.ignore_unknown_options = True
        self.default_cmd_name = kwargs.pop("default", None)
        self.default_if_no_args = kwargs.pop("default_if_no_args", False)
        super().__init__(*args, **kwargs)

Review comment (suggestion):
```suggestion
In particular, this class is what allows ``manim`` to act as ``manim render``.
```

Code after:
"""DefaultGroup allows a subcommand to act as the main command

In particular, this class is what allows ``manim`` to act as ``manim render``.
"""
import cloup

from .. import logger

__all__ = ["DefaultGroup"]


class DefaultGroup(cloup.Group):
    """Invokes a subcommand marked with ``default=True`` if any subcommand not
    chosen.
    """

    def __init__(self, *args, **kwargs):
        # To resolve as the default command.
        if not kwargs.get("ignore_unknown_options", True):
            raise ValueError("Default group accepts unknown options")
        self.ignore_unknown_options = True
        self.default_cmd_name = kwargs.pop("default", None)
        self.default_if_no_args = kwargs.pop("default_if_no_args", False)
        super().__init__(*args, **kwargs)

    def set_default_command(self, command):
        """Sets a command function as the default command."""
        cmd_name = command.name
Repository: ManimCommunity/manim
File: manim/cli/default_group.py (Python)

Code before:
"""DefaultGroup allows a subcommand to act as the main command

In particular, this class is what allows `manim` to act as `manim render`.
"""
import cloup

from .. import logger

__all__ = ["DefaultGroup"]


class DefaultGroup(cloup.Group):
    """Invokes a subcommand marked with `default=True` if any subcommand not
    chosen.

    :param default_if_no_args: resolves to the default command if no arguments
                               passed.

    """

    def __init__(self, *args, **kwargs):
        # To resolve as the default command.
        if not kwargs.get("ignore_unknown_options", True):
            raise ValueError("Default group accepts unknown options")
        self.ignore_unknown_options = True
        self.default_cmd_name = kwargs.pop("default", None)
        self.default_if_no_args = kwargs.pop("default_if_no_args", False)
        super().__init__(*args, **kwargs)

    def set_default_command(self, command):
        """Sets a command function as the default command."""
        cmd_name = command.name
        self.add_command(command)
        self.default_cmd_name = cmd_name

    def parse_args(self, ctx, args):
        if not args and self.default_if_no_args:
            args.insert(0, self.default_cmd_name)

Review comment (suggestion):
```suggestion
    """Invokes a subcommand marked with ``default=True`` if any subcommand not
```

Code after:
"""DefaultGroup allows a subcommand to act as the main command

In particular, this class is what allows ``manim`` to act as ``manim render``.
"""
import cloup

from .. import logger

__all__ = ["DefaultGroup"]


class DefaultGroup(cloup.Group):
    """Invokes a subcommand marked with ``default=True`` if any subcommand not
    chosen.
    """

    def __init__(self, *args, **kwargs):
        # To resolve as the default command.
        if not kwargs.get("ignore_unknown_options", True):
            raise ValueError("Default group accepts unknown options")
        self.ignore_unknown_options = True
        self.default_cmd_name = kwargs.pop("default", None)
        self.default_if_no_args = kwargs.pop("default_if_no_args", False)
        super().__init__(*args, **kwargs)

    def set_default_command(self, command):
        """Sets a command function as the default command."""
        cmd_name = command.name
        self.add_command(command)
        self.default_cmd_name = cmd_name

    def parse_args(self, ctx, args):
        if not args and self.default_if_no_args:
            args.insert(0, self.default_cmd_name)
        return super().parse_args(ctx, args)

    def get_command(self, ctx, cmd_name):
        if cmd_name not in self.commands:
Repository: ManimCommunity/manim
File: manim/plugins/import_plugins.py (Python)

Code before:
for plugin in pkg_resources.iter_entry_points("manim.plugins"):
    if plugin.name not in plugins_requested:
        continue
    loaded_plugin = plugin.load()
    if isinstance(loaded_plugin, types.ModuleType):
        # it is a module so it can't be called
        # see if __all__ is defined
        # if it is defined use that to load all the modules necessary
        # essentially this would be similar to `from plugin import *``
        # if not just import the module with the plugin name
        if hasattr(loaded_plugin, "__all__"):
            for thing in loaded_plugin.__all__:
                exec(f"{thing}=loaded_plugin.{thing}")
                __all__.append(thing)
        else:
            exec(f"{plugin.name}=loaded_plugin")
            __all__.append(plugin.name)
    elif isinstance(loaded_plugin, types.FunctionType):
        # call the function first
        # it will return a list of modules to add globally
        # finally add it
        lists = loaded_plugin()
        for lst in lists:
            exec(f"{lst.__name__}=lst")
            __all__.append(lst.__name__)
        plugins_requested.remove(plugin.name)

if plugins_requested != []:
    logger.warning("Missing Plugins: %s", plugins_requested)


Review comment (suggestion):
```suggestion
    plugins_requested.remove(plugin.name)
```

Code after:
for plugin in pkg_resources.iter_entry_points("manim.plugins"):
    if plugin.name not in plugins_requested:
        continue
    loaded_plugin = plugin.load()
    if isinstance(loaded_plugin, types.ModuleType):
        # it is a module so it can't be called
        # see if __all__ is defined
        # if it is defined use that to load all the modules necessary
        # essentially this would be similar to `from plugin import *``
        # if not just import the module with the plugin name
        if hasattr(loaded_plugin, "__all__"):
            for thing in loaded_plugin.__all__:
                exec(f"{thing}=loaded_plugin.{thing}")
                __all__.append(thing)
        else:
            exec(f"{plugin.name}=loaded_plugin")
            __all__.append(plugin.name)
    elif isinstance(loaded_plugin, types.FunctionType):
        # call the function first
        # it will return a list of modules to add globally
        # finally add it
        lists = loaded_plugin()
        for lst in lists:
            exec(f"{lst.__name__}=lst")
            __all__.append(lst.__name__)
    plugins_requested.remove(plugin.name)

if plugins_requested != []:
    logger.warning("Missing Plugins: %s", plugins_requested)
Repository: ManimCommunity/manim
File: manim/mobject/frame.py (Python)

Code before:

        When set, the width is stretched to accommodate
        the new aspect ratio.
        """

        return self.width / self.height

    @aspect_ratio.setter
    def aspect_ratio(self, value):
        self.stretch_to_fit_width(value * self.height)


class FullScreenRectangle(ScreenRectangle):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.height = config["frame_height"]


class FullScreenFadeRectangle(FullScreenRectangle):
    def __init__(self, stroke_width=0, fill_color=BLACK, fill_opacity=0.7, **kwargs):
        super().__init__(
            stroke_width=stroke_width,
            fill_color=fill_color,
            fill_opacity=fill_opacity,
            **kwargs
        )


class PictureInPictureFrame(Rectangle):
    def __init__(self, height=3, aspect_ratio=16.0 / 9.0, **kwargs):
        super().__init__(width=aspect_ratio * height, height=height, **kwargs)

    @property
    def aspect_ratio(self):
        """The aspect ratio.

        When set, the width is stretched to accommodate
        the new aspect ratio.
        """

        return self.width / self.height

    @aspect_ratio.setter
    def aspect_ratio(self, value):
        self.stretch_to_fit_width(value * self.height)


Review comment (suggestion):
```suggestion
@deprecated(
    since="v0.12.0",
    until="v0.13.0",
    replacement="FullScreenRectangle(stroke_width=0, fill_color=BLACK, fill_opacity=0.7)",
)
```

Code after:
    def aspect_ratio(self):
        """The aspect ratio.

        When set, the width is stretched to accommodate
        the new aspect ratio.
        """

        return self.width / self.height

    @aspect_ratio.setter
    def aspect_ratio(self, value):
        self.stretch_to_fit_width(value * self.height)


class FullScreenRectangle(ScreenRectangle):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.height = config["frame_height"]


@deprecated(
    since="v0.12.0",
    until="v0.13.0",
    message="This method is deprecated due to decluttering purpose.",
    replacement="FullScreenRectangle(stroke_width=0, fill_color=BLACK, fill_opacity=0.7)",
)
class FullScreenFadeRectangle(FullScreenRectangle):
    def __init__(self, stroke_width=0, fill_color=BLACK, fill_opacity=0.7, **kwargs):
        super().__init__(
            stroke_width=stroke_width,
            fill_color=fill_color,
            fill_opacity=fill_opacity,
            **kwargs
        )


@deprecated(
    since="v0.12.0",
    until="v0.13.0",
    message="This method is deprecated due to decluttering purpose.",
)
class PictureInPictureFrame(Rectangle):
    def __init__(self, height=3, aspect_ratio=16.0 / 9.0, **kwargs):
        super().__init__(width=aspect_ratio * height, height=height, **kwargs)

    @property
    def aspect_ratio(self):
        """The aspect ratio.

        When set, the width is stretched to accommodate
        the new aspect ratio.
Repository: ManimCommunity/manim
File: tests/test_auto_zoom.py (Python)

Code before:
from manim import *

from .simple_zoom_scene import test_zoom_pan_to_center


def test_zoom():

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

    scene = test_zoom_pan_to_center()

    scene.construct()

    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 (suggestion):
I am slightly confused about this test, and I don't think this is a good pattern. My suggestion would be to do something along the lines of

```
def test_auto_zoom():

    s1 = Square()
    s1.set_x(-10)
    s2 = Square()
    s2.set_x(10)

    scene = MovingCameraScene()
    scene.add(s1, s2)
    scene.play(scene.camera.auto_zoom([s1, s2])
```

and remove the introduced auxiliary file. (I think it is not good that the squares in the auxiliary file and this test have to be the same; there should just be a single source of truth.)

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: manim/camera/moving_camera.py (Python)

Code before:
"""A camera able to move through a scene.

.. SEEALSO::

    :mod:`.moving_camera_scene`

"""

__all__ = ["CameraFrame", "MovingCamera"]


import math
import operator

from numpy import string_

from .. import config
from ..camera.camera import Camera
from ..constants import *
from ..mobject.frame import ScreenRectangle
from ..mobject.types.vectorized_mobject import VGroup
from ..utils.color import WHITE


# TODO, think about how to incorporate perspective
class CameraFrame(VGroup):
    def __init__(self, center=ORIGIN, **kwargs):
        super().__init__(center=center, **kwargs)
        self.width = config["frame_width"]
        self.height = config["frame_height"]


class MovingCamera(Camera):
    """
    Stays in line with the height, width and position of it's 'frame', which is a Rectangle

    .. SEEALSO::

        :class:`.MovingCameraScene`

    """

    def __init__(
        self,

Review comment (suggestion):
I don't like *-imports in library files.
```suggestion
from ..constants import LEFT, RIGHT, UP, DOWN, ORIGIN
```

Code after:
"""A camera able to move through a scene.

.. SEEALSO::

    :mod:`.moving_camera_scene`

"""

__all__ = ["CameraFrame", "MovingCamera"]

from .. import config
from ..camera.camera import Camera
from ..constants import DOWN, LEFT, ORIGIN, RIGHT, UP
from ..mobject.frame import ScreenRectangle
from ..mobject.types.vectorized_mobject import VGroup
from ..utils.color import WHITE


# TODO, think about how to incorporate perspective
class CameraFrame(VGroup):
    def __init__(self, center=ORIGIN, **kwargs):
        super().__init__(center=center, **kwargs)
        self.width = config["frame_width"]
        self.height = config["frame_height"]


class MovingCamera(Camera):
    """
    Stays in line with the height, width and position of it's 'frame', which is a Rectangle

    .. SEEALSO::

        :class:`.MovingCameraScene`

    """

    def __init__(
        self,
        frame=None,
        fixed_dimension=0,  # width
        default_frame_stroke_color=WHITE,
        default_frame_stroke_width=0,
        **kwargs
    ):
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)

    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):
We should make sure that nothing is actually rendered.

```suggestion
    with tempconfig({'dry_run': True}):
        scene = MovingCameraScene()
        scene.add(s1, s2)
        scene.play(scene.camera.auto_zoom([s1, s2]))
```

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)
    )