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

Code before:
import sys

import pytest

from ..utils.commands import capture


@pytest.mark.slow
def test_unicode_partial_movie(tmpdir, simple_scenes_path):
    # Characters that failed for a user on Windows
    # due to its weird default encoding.
    unicode_str = "三角函数"

    scene_name = "SquareToCircle"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--media_dir",
        str(tmpdir / unicode_str),
        simple_scenes_path,
        scene_name,
    ]

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


Review comment (suggestion):
```suggestion
from manim.utils.commands import capture
```

Code after:
import sys

import pytest

from manim.utils.commands import capture


@pytest.mark.slow
def test_unicode_partial_movie(tmpdir, simple_scenes_path):
    # Characters that failed for a user on Windows
    # due to its weird default encoding.
    unicode_str = "三角函数"

    scene_name = "SquareToCircle"
    command = [
        sys.executable,
        "-m",
        "manim",
        "--media_dir",
        str(tmpdir / unicode_str),
        simple_scenes_path,
        scene_name,
    ]

    _, err, exit_code = capture(command)
    assert exit_code == 0, err
Repository: ManimCommunity/manim
File: manim/mobject/svg/brace.py (Python)

Code before:
                    arc_4 = Arc(radius=0.2,start_angle=0,angle=3*PI/2).set_color(GREEN)
                    brace_4 = ArcBrace(arc_4)
                    group_4 = VGroup(arc_4,brace_4)

                    arc_group = VGroup(group_1, group_2, group_3, group_4).arrange_in_grid(buff=1.5)
                    self.add(arc_group.center())

    """

    def __init__(
        self, arc=Arc(start_angle=-1, angle=2, radius=1), direction=RIGHT, **kwargs
    ):
        arc_center = arc.get_arc_center()
        arc_start_angle = arc.start_angle
        arc_end_angle = arc_start_angle + arc.angle
        arc_radius = arc.radius
        scale_shift = RIGHT * np.log(arc_radius)
        line = Line(UP * arc_start_angle, UP * arc_end_angle)
        if arc_radius >= 1:
            line.scale(arc_radius, about_point=ORIGIN)
        Brace.__init__(self, line, direction=direction, **kwargs)
        if arc_radius >= 1:
            self.scale(1 / (arc_radius), about_point=ORIGIN)
        self.shift(scale_shift)
        self.apply_complex_function(np.exp)
        self.shift(arc_center)


Review comment (suggestion):
Added spacing to make the code more legible.
```suggestion
        arc_center = arc.get_arc_center()
        arc_start_angle = arc.start_angle
        arc_end_angle = arc_start_angle + arc.angle
        arc_radius = arc.radius
        
        scale_shift = RIGHT * np.log(arc_radius)
        line = Line(UP * arc_start_angle, UP * arc_end_angle)
        
        if arc_radius >= 1:
            line.scale(arc_radius, about_point=ORIGIN)
        Brace.__init__(self, line, direction=direction, **kwargs)
        
        if arc_radius >= 1:
            self.scale(1 / (arc_radius), about_point=ORIGIN)
            
        self.shift(scale_shift)
        self.apply_complex_function(np.exp)
        self.shift(arc_center)
```

Code after:

                    arc_group = VGroup(group_1, group_2, group_3, group_4).arrange_in_grid(buff=1.5)
                    self.add(arc_group.center())

    """

    def __init__(
        self,
        arc: Arc = Arc(start_angle=-1, angle=2, radius=1),
        direction: Sequence[float] = RIGHT,
        **kwargs
    ):
        arc_end_angle = arc.start_angle + arc.angle
        line = Line(UP * arc.start_angle, UP * arc_end_angle)
        scale_shift = RIGHT * np.log(arc.radius)

        if arc.radius >= 1:
            line.scale(arc.radius, about_point=ORIGIN)
            Brace.__init__(self, line, direction=direction, **kwargs)
            self.scale(1 / (arc.radius), about_point=ORIGIN)
        else:
            Brace.__init__(self, line, direction=direction, **kwargs)

        if arc.radius >= 0.3:
            self.shift(scale_shift)
        else:
            self.shift(RIGHT * np.log(0.3))

        self.apply_complex_function(np.exp)
        self.shift(arc.get_arc_center())
Repository: ManimCommunity/manim
File: manim/mobject/svg/brace.py (Python)

Code before:
    """

    def __init__(
        self,
        arc: Arc = Arc(start_angle=-1, angle=2, radius=1),
        direction: Sequence[float] = RIGHT,
        **kwargs
    ):
        arc_center = arc.get_arc_center()
        arc_end_angle = arc.start_angle + arc.angle

        scale_shift = RIGHT * np.log(arc.radius)
        line = Line(UP * arc.start_angle, UP * arc_end_angle)

        if arc.radius >= 1:
            line.scale(arc.radius)
        Brace.__init__(self, line, direction=direction, **kwargs)
        if arc.radius >= 1:
            self.scale(1 / (arc.radius))

        if arc.radius >= 0.3:
            self.shift(scale_shift)
        else:
            self.shift(RIGHT * np.log(0.3))
        self.apply_complex_function(np.exp)
        self.shift(arc_center)


Review comment (suggestion):
Removes the variable `arc_center` and just uses the function when it needs to be called. Restores `about_point=ORIGIN` since this was actually needed for non center_centered arcs (docs look off):
![image](https://user-images.githubusercontent.com/70682032/119311795-81b4bf80-bc3f-11eb-9f36-c6feed0938b2.png)
Re-formats some of the code to make it more explicit in its purpose.

```suggestion
        arc_end_angle = arc.start_angle + arc.angle
        line = Line(UP * arc.start_angle, UP * arc_end_angle)
        scale_shift = RIGHT * np.log(arc.radius)
        
        if arc.radius >= 1:
            line.scale(arc.radius, about_point=ORIGIN)
            Brace.__init__(self, line, direction=direction, **kwargs)
            self.scale(1 / (arc.radius), about_point=ORIGIN)
        else:
            Brace.__init__(self, line, direction=direction, **kwargs)
            
        if arc.radius >= 0.3:
            self.shift(scale_shift)
        else:
            self.shift(RIGHT * np.log(0.3))
            
        self.apply_complex_function(np.exp)
        self.shift(arc.get_arc_center())
```

Code after:
    """

    def __init__(
        self,
        arc: Arc = Arc(start_angle=-1, angle=2, radius=1),
        direction: Sequence[float] = RIGHT,
        **kwargs
    ):
        arc_end_angle = arc.start_angle + arc.angle
        line = Line(UP * arc.start_angle, UP * arc_end_angle)
        scale_shift = RIGHT * np.log(arc.radius)

        if arc.radius >= 1:
            line.scale(arc.radius, about_point=ORIGIN)
            Brace.__init__(self, line, direction=direction, **kwargs)
            self.scale(1 / (arc.radius), about_point=ORIGIN)
        else:
            Brace.__init__(self, line, direction=direction, **kwargs)

        if arc.radius >= 0.3:
            self.shift(scale_shift)
        else:
            self.shift(RIGHT * np.log(0.3))

        self.apply_complex_function(np.exp)
        self.shift(arc.get_arc_center())
Repository: ManimCommunity/manim
File: manim/mobject/opengl_compatibility.py (Python)

Code before:
from abc import ABCMeta

from .. import config

# from .mobject import Mobject # cannot import since this is cyclic
from .opengl_mobject import OpenGLMobject
from .types.opengl_vectorized_mobject import OpenGLVMobject


class ConvertToOpenGL(ABCMeta):
    """Metaclass for swapping (V)Mobject with its OpenGL counterpart at runtime
    depending on config.renderer. This metaclass should only need to be inherited
    on the lowest order inheritance classes such as Mobject and VMobject.

    Note that with this implementation, changing the value of ``config.renderer``
    after Manim has been imported won't have the desired effect and will lead to
    spurious errors.
    """

    def __new__(cls, name, bases, namespace):
        if config.renderer == "opengl":
            # must check class names to prevent
            #  cyclic importing
            baseNameDict = {
                "Mobject": OpenGLMobject,
                "VMobject": OpenGLVMobject,
            }

            bases = tuple((baseNameDict.get(base.__name__, base) for base in bases))


Review comment (suggestion):
I don't think it is necessary to keep this comment around.

```suggestion
```

Code after:
from abc import ABCMeta

from .. import config
from .opengl_mobject import OpenGLMobject
from .types.opengl_vectorized_mobject import OpenGLVMobject


class ConvertToOpenGL(ABCMeta):
    """Metaclass for swapping (V)Mobject with its OpenGL counterpart at runtime
    depending on config.renderer. This metaclass should only need to be inherited
    on the lowest order inheritance classes such as Mobject and VMobject.

    Note that with this implementation, changing the value of ``config.renderer``
    after Manim has been imported won't have the desired effect and will lead to
    spurious errors.
    """

    def __new__(cls, name, bases, namespace):
        if config.renderer == "opengl":
            # Must check class names to prevent
            # cyclic importing.
            base_names_to_opengl = {
                "Mobject": OpenGLMobject,
                "VMobject": OpenGLVMobject,
            }

            bases = tuple(
                base_names_to_opengl.get(base.__name__, base) for base in bases
            )
Repository: ManimCommunity/manim
File: manim/mobject/opengl_compatibility.py (Python)

Code before:

from .. import config
from .opengl_mobject import OpenGLMobject
from .types.opengl_vectorized_mobject import OpenGLVMobject


class ConvertToOpenGL(ABCMeta):
    """Metaclass for swapping (V)Mobject with its OpenGL counterpart at runtime
    depending on config.renderer. This metaclass should only need to be inherited
    on the lowest order inheritance classes such as Mobject and VMobject.

    Note that with this implementation, changing the value of ``config.renderer``
    after Manim has been imported won't have the desired effect and will lead to
    spurious errors.
    """

    def __new__(cls, name, bases, namespace):
        if config.renderer == "opengl":
            # must check class names to prevent
            #  cyclic importing
            baseNameDict = {
                "Mobject": OpenGLMobject,
                "VMobject": OpenGLVMobject,
            }

            bases = tuple((baseNameDict.get(base.__name__, base) for base in bases))

        return super().__new__(cls, name, bases, namespace)


Review comment (suggestion):
```suggestion
            # Must check class names to prevent
            # cyclic importing.
            base_names_to_opengl = {
                "Mobject": OpenGLMobject,
                "VMobject": OpenGLVMobject,
            }

            bases = tuple(base_names_to_opengl.get(base.__name__, base) for base in bases)
```

Code after:

from .. import config
from .opengl_mobject import OpenGLMobject
from .types.opengl_vectorized_mobject import OpenGLVMobject


class ConvertToOpenGL(ABCMeta):
    """Metaclass for swapping (V)Mobject with its OpenGL counterpart at runtime
    depending on config.renderer. This metaclass should only need to be inherited
    on the lowest order inheritance classes such as Mobject and VMobject.

    Note that with this implementation, changing the value of ``config.renderer``
    after Manim has been imported won't have the desired effect and will lead to
    spurious errors.
    """

    def __new__(cls, name, bases, namespace):
        if config.renderer == "opengl":
            # Must check class names to prevent
            # cyclic importing.
            base_names_to_opengl = {
                "Mobject": OpenGLMobject,
                "VMobject": OpenGLVMobject,
            }

            bases = tuple(
                base_names_to_opengl.get(base.__name__, base) for base in bases
            )

        return super().__new__(cls, name, bases, namespace)
Repository: ManimCommunity/manim
File: manim/mobject/three_dimensions.py (Python)

Code before:
            **kwargs,
        )

    def generate_points(self):
        for vect in IN, OUT, LEFT, RIGHT, UP, DOWN:
            face = Square(
                side_length=self.side_length,
                shade_in_3d=True,
            )
            face.flip()
            face.shift(self.side_length * OUT / 2.0)
            face.apply_matrix(z_to_vector(vect))

            self.add(face)


class Prism(Cube):
    """A Prism.

    Examples
    --------

    .. manim:: ExamplePrism
        :save_last_frame:

        class ExamplePrism(Scene):
            def construct(self):
                prismSmall = Prism(dimensions=[1,2,3]).rotate(PI/2)
                prismLarge = Prism(dimensions=[2,4,6]).move_to([3,0,0])
                self.add(prismSmall,prismLarge)
    """

    def __init__(self, dimensions=[3, 2, 1], **kwargs):
        self.dimensions = dimensions
        Cube.__init__(self, **kwargs)

    def generate_points(self):
        Cube.generate_points(self)
        for dim, value in enumerate(self.dimensions):
            self.rescale_to_fit(value, dim, stretch=True)


class Cone(ParametricSurface):
    """A circular cone.
    Can be defined using 2 parameters: its height, and its base radius.
    The polar angle, theta, can be calculated using arctan(base_radius /
    height) The spherical radius, r, is calculated using the pythagorean
    theorem.

    Examples
    --------

Review comment (suggestion):
```suggestion
        class ExamplePrism(ThreeDScene):
```

Code after:
                dot_2 = Dot3D(point=axes.coords_to_point(2, 0, 0), radius=0.1, color=BLUE)
                dot_3 = Dot3D(point=[0, 0, 0], radius=0.1, color=ORANGE)
                self.add(axes, dot_1, dot_2,dot_3)
    """

    def __init__(self, point=ORIGIN, radius=DEFAULT_DOT_RADIUS, color=WHITE, **kwargs):
        Sphere.__init__(self, center=point, radius=radius, **kwargs)
        self.set_color(color)


class Cube(VGroup):
    def __init__(
        self,
        side_length=2,
        fill_opacity=0.75,
        fill_color=BLUE,
        stroke_width=0,
        **kwargs
    ):
        self.side_length = side_length
        super().__init__(
            fill_color=fill_color,
            fill_opacity=fill_opacity,
            stroke_width=stroke_width,
            **kwargs,
        )

    def generate_points(self):
        for vect in IN, OUT, LEFT, RIGHT, UP, DOWN:
            face = Square(
                side_length=self.side_length,
                shade_in_3d=True,
            )
            face.flip()
            face.shift(self.side_length * OUT / 2.0)
            face.apply_matrix(z_to_vector(vect))

            self.add(face)


class Prism(Cube):
    """A cuboid.

    Examples
    --------

    .. manim:: ExamplePrism
        :save_last_frame:

        class ExamplePrism(ThreeDScene):
            def construct(self):
Repository: ManimCommunity/manim
File: manim/mobject/three_dimensions.py (Python)

Code before:
            face.set_fill(colors[c_index], opacity=opacity)
        return self


# Specific shapes


class Sphere(ParametricSurface):
    """A sphere

    Examples
    ---------
    .. manim:: ExampleSphere
        :save_last_frame:

        class ExampleSphere(ThreeDScene):
            def construct(self):
                self.set_camera_orientation(phi = PI/6, theta = PI/6)
                sphere1 = Sphere(center = (3,0,0),radius = 1,resolution = (30, 40),u_min = 0.001,u_max = PI - 0.001,v_min = 0,v_max = TAU)
                sphere1.set_color(RED)
                self.add(sphere1)
                sphere2 = Sphere(center=(0,1,0),radius=2,resolution=(40, 50))
                sphere2.move_to((-1,-3,0))
                sphere2.set_color(GREEN)
                self.add(sphere2)
                sphere3 = Sphere(center=(0,1,0),radius=2,resolution=(40, 50))
                sphere3.move_to((-1,2,0))
                sphere3.set_color(BLUE)
                self.add(sphere3)

    """

    def __init__(
        self,
        center=ORIGIN,
        radius=1,
        resolution=(12, 24),
        u_min=0.001,
        u_max=PI - 0.001,
        v_min=0,
        v_max=TAU,
        **kwargs
    ):
        ParametricSurface.__init__(
            self,
            self.func,
            resolution=resolution,
            u_min=u_min,
            u_max=u_max,
            v_min=v_min,
            v_max=v_max,

Review comment (suggestion):
All these examples are pretty high res. I would recommend showing what a low res sphere would look like too. This is a sample value I toyed around with. 
```suggestion
                sphere3 = Sphere(center=(0,1,0),radius=2,resolution=(14, 14))
```

Code after:
            face.set_fill(colors[c_index], opacity=opacity)
        return self


# Specific shapes


class Sphere(ParametricSurface):
    """A mobject representing a three-dimensional sphere.

    Examples
    ---------

    .. manim:: ExampleSphere
        :save_last_frame:

        class ExampleSphere(ThreeDScene):
            def construct(self):
                self.set_camera_orientation(phi=PI / 6, theta=PI / 6)
                sphere1 = Sphere(
                    center=(3, 0, 0),
                    radius=1,
                    resolution=(20, 20),
                    u_min=0.001,
                    u_max=PI - 0.001,
                    v_min=0,
                    v_max=TAU,
                )
                sphere1.set_color(RED)
                self.add(sphere1)
                sphere2 = Sphere(center=(-1, -3, 0), radius=2, resolution=(18, 18))
                sphere2.set_color(GREEN)
                self.add(sphere2)
                sphere3 = Sphere(center=(-1, 2, 0), radius=2, resolution=(16, 16))
                sphere3.set_color(BLUE)
                self.add(sphere3)
    """

    def __init__(
        self,
        center=ORIGIN,
        radius=1,
        resolution=(12, 24),
        u_min=0.001,
        u_max=PI - 0.001,
        v_min=0,
        v_max=TAU,
        **kwargs
    ):
        ParametricSurface.__init__(
            self,
Repository: ManimCommunity/manim
File: manim/mobject/probability.py (Python)

Code before:
from ..mobject.types.vectorized_mobject import VGroup
from ..utils.color import (
    BLUE,
    BLUE_E,
    DARK_GREY,
    GREEN_E,
    LIGHT_GREY,
    MAROON_B,
    YELLOW,
    color_gradient,
)
from ..utils.iterables import tuplify

EPSILON = 0.0001


class SampleSpace(Rectangle):
    """A sample space.

    Examples
    --------
    .. manim:: ExampleSampleSpace
        :save_last_frame:
        class ExampleSampleSpace(Scene):
            def construct(self):
                poly1 = SampleSpace(stroke_width=15,fill_opacity=1).move_to([-1,0,0])
                poly2 = SampleSpace(width = 5, height = 3, stroke_width=5,fill_opacity=0.5).move_to([4,0,0])
                poly3 = SampleSpace(width = 2, height = 2, stroke_width=5,fill_opacity=0.1).move_to([-4.5,0,0])
                self.add(poly1)
                self.add(poly2)
                self.add(poly3)

    """

    def __init__(
        self,
        height=3,
        width=3,
        fill_color=DARK_GREY,
        fill_opacity=1,
        stroke_width=0.5,
        stroke_color=LIGHT_GREY,
        default_label_scale_val=1,
    ):
        Rectangle.__init__(
            self,
            height=height,
            width=width,
            fill_color=fill_color,
            fill_opacity=fill_opacity,
            stroke_width=stroke_width,

Review comment (suggestion):
```suggestion

        class ExampleSampleSpace(Scene):
```

Code after:
from ..mobject.types.vectorized_mobject import VGroup
from ..utils.color import (
    BLUE,
    BLUE_E,
    DARK_GREY,
    GREEN_E,
    LIGHT_GREY,
    MAROON_B,
    YELLOW,
    color_gradient,
)
from ..utils.iterables import tuplify

EPSILON = 0.0001


class SampleSpace(Rectangle):
    """

    Examples
    --------

    .. manim:: ExampleSampleSpace
        :save_last_frame:

        class ExampleSampleSpace(Scene):
            def construct(self):
                poly1 = SampleSpace(stroke_width=15, fill_opacity=1)
                poly2 = SampleSpace(width=5, height=3, stroke_width=5, fill_opacity=0.5)
                poly3 = SampleSpace(width=2, height=2, stroke_width=5, fill_opacity=0.1)
                poly3.divide_vertically(p_list=np.array([0.37, 0.13, 0.5]), colors=[BLACK, WHITE, GRAY], vect=RIGHT)
                poly_group = VGroup(poly1, poly2, poly3).arrange()
                self.add(poly_group)
    """

    def __init__(
        self,
        height=3,
        width=3,
        fill_color=DARK_GREY,
        fill_opacity=1,
        stroke_width=0.5,
        stroke_color=LIGHT_GREY,
        default_label_scale_val=1,
    ):
        Rectangle.__init__(
            self,
            height=height,
            width=width,
            fill_color=fill_color,
            fill_opacity=fill_opacity,
Repository: ManimCommunity/manim
File: manim/mobject/probability.py (Python)

Code before:
    MAROON_B,
    YELLOW,
    color_gradient,
)
from ..utils.iterables import tuplify

EPSILON = 0.0001


class SampleSpace(Rectangle):
    """A sample space.

    Examples
    --------
    .. manim:: ExampleSampleSpace
        :save_last_frame:

        class ExampleSampleSpace(Scene):
            def construct(self):
                poly1 = SampleSpace(stroke_width=15,fill_opacity=1)
                poly2 = SampleSpace(width = 5, height = 3, stroke_width=5,fill_opacity=0.5)
                poly3 = SampleSpace(width = 2, height = 2, stroke_width=5,fill_opacity=0.1)
                poly3.divide_vertically( p_list= np.array([0.37,0.13,0.5]), colors=[BLACK, WHITE,GRAY], vect=RIGHT)
                poly_group = VGroup()
                poly_group.add(*[poly1,poly2,poly3]).arrange()
                self.add(poly_group)

    """

    def __init__(
        self,
        height=3,
        width=3,
        fill_color=DARK_GREY,
        fill_opacity=1,
        stroke_width=0.5,
        stroke_color=LIGHT_GREY,
        default_label_scale_val=1,
    ):
        Rectangle.__init__(
            self,
            height=height,
            width=width,
            fill_color=fill_color,
            fill_opacity=fill_opacity,
            stroke_width=stroke_width,
            stroke_color=stroke_color,
        )
        self.default_label_scale_val = default_label_scale_val

    def add_title(self, title="Sample space", buff=MED_SMALL_BUFF):

Review comment (suggestion):
```suggestion
                poly_group = VGroup(poly1,poly2,poly3).arrange()
                self.add(poly_group)
```

Code after:
    MAROON_B,
    YELLOW,
    color_gradient,
)
from ..utils.iterables import tuplify

EPSILON = 0.0001


class SampleSpace(Rectangle):
    """

    Examples
    --------

    .. manim:: ExampleSampleSpace
        :save_last_frame:

        class ExampleSampleSpace(Scene):
            def construct(self):
                poly1 = SampleSpace(stroke_width=15, fill_opacity=1)
                poly2 = SampleSpace(width=5, height=3, stroke_width=5, fill_opacity=0.5)
                poly3 = SampleSpace(width=2, height=2, stroke_width=5, fill_opacity=0.1)
                poly3.divide_vertically(p_list=np.array([0.37, 0.13, 0.5]), colors=[BLACK, WHITE, GRAY], vect=RIGHT)
                poly_group = VGroup(poly1, poly2, poly3).arrange()
                self.add(poly_group)
    """

    def __init__(
        self,
        height=3,
        width=3,
        fill_color=DARK_GREY,
        fill_opacity=1,
        stroke_width=0.5,
        stroke_color=LIGHT_GREY,
        default_label_scale_val=1,
    ):
        Rectangle.__init__(
            self,
            height=height,
            width=width,
            fill_color=fill_color,
            fill_opacity=fill_opacity,
            stroke_width=stroke_width,
            stroke_color=stroke_color,
        )
        self.default_label_scale_val = default_label_scale_val

    def add_title(self, title="Sample space", buff=MED_SMALL_BUFF):
        # TODO, should this really exist in SampleSpaceScene