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

Code before:
            Line(UP + LEFT, DOWN + RIGHT),
            Line(UP + RIGHT, DOWN + LEFT),
        )
        self.replace(mobject, stretch=True)
        self.set_stroke(color=stroke_color, width=stroke_width)


class Underline(Line):
    """Creates an underline.

    Parameters
    ----------
    Line
        The underline.

    Examples
    --------
    .. manim:: UnderLine

        class UnderLine(Scene):
            def construct(self):
                m = Tex("M")  # Starting letter
                man = Tex("M", "anim")  # Full Word
                ul = Underline(man)  # Underlining the word
                self.play(Write(m), run_time=1.5)
                self.play(ReplacementTransform(m, man[0]), FadeIn(man[1]))  # Replaces the starting letter to word
                self.play(Create(ul))  # Creates the underline
    """

    def __init__(self, mobject, buff=SMALL_BUFF, **kwargs):
        super().__init__(LEFT, RIGHT, buff=buff, **kwargs)
        self.match_width(mobject)
        self.next_to(mobject, DOWN, buff=self.buff)


Review comment (suggestion):
In general, I think the example is fine but a bit too simple to be a video (which is not a bad thing -- simple is good); if you *insist* on making it a video it could be made a super short 1 second clip where the setup for the word Manim is done in advance and the act of underlining is the *only* thing animated e.g.:

```suggestion
                man = Tex("M", "anim")  # Full Word
                self.add(man)
                ul = Underline(man)  # Underlining the word
```

Or better yet, it could just be made an image using `:save_last_frame:`.

Code after:
            Line(UP + LEFT, DOWN + RIGHT),
            Line(UP + RIGHT, DOWN + LEFT),
        )
        self.replace(mobject, stretch=True)
        self.set_stroke(color=stroke_color, width=stroke_width)


class Underline(Line):
    """Creates an underline.

    Parameters
    ----------
    Line
        The underline.

    Examples
    --------
    .. manim:: UnderLine
        :save_last_frame:

        class UnderLine(Scene):
            def construct(self):
                man = Tex("Manim")  # Full Word
                ul = Underline(man)  # Underlining the word
                self.add(man, ul)
    """

    def __init__(self, mobject, buff=SMALL_BUFF, **kwargs):
        super().__init__(LEFT, RIGHT, buff=buff, **kwargs)
        self.match_width(mobject)
        self.next_to(mobject, DOWN, buff=self.buff)
Repository: ManimCommunity/manim
File: manim/mobject/geometry.py (Python)

Code before:
        ]
        outer_arc.reverse_points()
        self.append_points(inner_arc.get_points())
        self.add_line_to(outer_arc.get_points()[0])
        self.append_points(outer_arc.get_points())
        self.add_line_to(inner_arc.get_points()[0])

    init_points = generate_points


class Sector(AnnularSector):
    """

    Examples
    --------

    .. manim:: ExampleSector
        :save_last_frame:

        class ExampleSector(Scene):
            def construct(self):
                sector = Sector(outer_radius=2, inner_radius=1)
                sector2 = Sector(outer_radius=2.5, inner_radius=0.8).move_to([-3, 0, 0])
                sector.set_color(RED)
                sector2.set_color(PINK)
                self.add(sector,sector2)
    """

    def __init__(self, outer_radius=1, inner_radius=0, **kwargs):
        super().__init__(inner_radius=inner_radius, outer_radius=outer_radius, **kwargs)


class Annulus(Circle):
    """Region between two concentric :class:`Circles <.Circle>`.

    Parameters
    ----------
    inner_radius
        The radius of the inner :class:`Circle`.
    outer_radius
        The radius of the outer :class:`Circle`.
    kwargs : Any
        Additional arguments to be passed to :class:`Annulus`

    Examples
    --------

    .. manim:: AnnulusExample
        :save_last_frame:

        class AnnulusExample(Scene):

Review comment (suggestion):
```suggestion
                self.add(sector, sector2)
```

Code after:
            fill_opacity=fill_opacity,
            stroke_width=stroke_width,
            color=color,
            **kwargs,
        )

    def generate_points(self):
        inner_arc, outer_arc = [
            Arc(
                start_angle=self.start_angle,
                angle=self.angle,
                radius=radius,
                arc_center=self.arc_center,
            )
            for radius in (self.inner_radius, self.outer_radius)
        ]
        outer_arc.reverse_points()
        self.append_points(inner_arc.get_points())
        self.add_line_to(outer_arc.get_points()[0])
        self.append_points(outer_arc.get_points())
        self.add_line_to(inner_arc.get_points()[0])

    init_points = generate_points


class Sector(AnnularSector):
    """

    Examples
    --------

    .. manim:: ExampleSector
        :save_last_frame:

        class ExampleSector(Scene):
            def construct(self):
                sector = Sector(outer_radius=2, inner_radius=1)
                sector2 = Sector(outer_radius=2.5, inner_radius=0.8).move_to([-3, 0, 0])
                sector.set_color(RED)
                sector2.set_color(PINK)
                self.add(sector, sector2)
    """

    def __init__(self, outer_radius=1, inner_radius=0, **kwargs):
        super().__init__(inner_radius=inner_radius, outer_radius=outer_radius, **kwargs)


class Annulus(Circle):
    """Region between two concentric :class:`Circles <.Circle>`.

    Parameters
Repository: ManimCommunity/manim
File: manim/utils/ipython_magic.py (Python)

Code before:

                if local_path in self.rendered_files:
                    self.rendered_files[local_path].unlink()
                self.rendered_files[local_path] = tmpfile
                os.makedirs(tmpfile.parent, exist_ok=True)
                shutil.copy(local_path, tmpfile)

                file_type = mimetypes.guess_type(config["output_file"])[0]
                if file_type.startswith("image"):
                    display(Image(filename=config["output_file"]))
                    return

                # videos need to be embedded when running in google colab
                video_embed = "google.colab" in str(get_ipython())

                display(
                    Video(
                        tmpfile,
                        html_attributes=f'controls autoplay loop style="max-width: {config["media_width"]};"',
                        embed=video_embed,
                    )
                )


def _video_hash(path):
    sha1 = hashlib.sha1()
    with open(path, "rb") as f:
        while True:
            data = f.read(65536)
            if not data:
                break
            sha1.update(data)
    return sha1.hexdigest()


Review comment (suggestion):
```suggestion
    return config["scene_names"][0] + "@" + datetime.now().strftime("%Y-%m-%d@%H-%M-%S")
```

Code after:

                if local_path in self.rendered_files:
                    self.rendered_files[local_path].unlink()
                self.rendered_files[local_path] = tmpfile
                os.makedirs(tmpfile.parent, exist_ok=True)
                shutil.copy(local_path, tmpfile)

                file_type = mimetypes.guess_type(config["output_file"])[0]
                if file_type.startswith("image"):
                    display(Image(filename=config["output_file"]))
                    return

                # videos need to be embedded when running in google colab
                video_embed = "google.colab" in str(get_ipython())

                display(
                    Video(
                        tmpfile,
                        html_attributes=f'controls autoplay loop style="max-width: {config["media_width"]};"',
                        embed=video_embed,
                    )
                )


def _generate_file_name():
    return config["scene_names"][0] + "@" + datetime.now().strftime("%Y-%m-%d@%H-%M-%S")
Repository: ManimCommunity/manim
File: manim/utils/ipython_magic.py (Python)

Code before:

                if local_path in self.rendered_files:
                    self.rendered_files[local_path].unlink()
                self.rendered_files[local_path] = tmpfile
                os.makedirs(tmpfile.parent, exist_ok=True)
                shutil.copy(local_path, tmpfile)

                file_type = mimetypes.guess_type(config["output_file"])[0]
                if file_type.startswith("image"):
                    display(Image(filename=config["output_file"]))
                    return

                # videos need to be embedded when running in google colab
                video_embed = "google.colab" in str(get_ipython())

                display(
                    Video(
                        tmpfile,
                        html_attributes=f'controls autoplay loop style="max-width: {config["media_width"]};"',
                        embed=video_embed,
                    )
                )


def _video_hash(path):
    sha1 = hashlib.sha1()
    with open(path, "rb") as f:
        while True:
            data = f.read(65536)
            if not data:
                break
            sha1.update(data)
    return sha1.hexdigest()


Review comment (suggestion):
```suggestion
    return config["scene_names"][0] + "@" + datetime.now().strftime("%y-%m-%d@%H-%M-%S")
```
got this from here:
![image](https://user-images.githubusercontent.com/44469195/121228827-84ddbb80-c88d-11eb-8448-f4de091de702.png)



Code after:

                if local_path in self.rendered_files:
                    self.rendered_files[local_path].unlink()
                self.rendered_files[local_path] = tmpfile
                os.makedirs(tmpfile.parent, exist_ok=True)
                shutil.copy(local_path, tmpfile)

                file_type = mimetypes.guess_type(config["output_file"])[0]
                if file_type.startswith("image"):
                    display(Image(filename=config["output_file"]))
                    return

                # videos need to be embedded when running in google colab
                video_embed = "google.colab" in str(get_ipython())

                display(
                    Video(
                        tmpfile,
                        html_attributes=f'controls autoplay loop style="max-width: {config["media_width"]};"',
                        embed=video_embed,
                    )
                )


def _generate_file_name():
    return config["scene_names"][0] + "@" + datetime.now().strftime("%Y-%m-%d@%H-%M-%S")
Repository: ManimCommunity/manim
File: tests/test_ipython_magic.py (Python)

Code before:
import re

from manim import tempconfig
from manim.utils.ipython_magic import _generate_file_name


def test_jupyter_file_naming():
    """Check the format of file names for jupyter"""
    scene_name = "SimpleScene"
    expected_pattern = r"[0-9a-zA-Z@_-]+[@_-]\d\d\d\d-\d\d-\d\d[@_-]\d\d-\d\d-\d\d"
    with tempconfig({"scene_names": [scene_name]}):
        file_name = _generate_file_name()
        match = re.match(expected_pattern, file_name)
        assert scene_name in file_name, (
            "Expected file to contain " + scene_name + " but got " + file_name
        )
        assert match, "file name does not match expected pattern " + expected_pattern


def test_jupyter_file_output(tmp_path):
    """Check the jupyter file naming is valid and can be created"""
    scene_name = "SimpleScene"
    with tempconfig({"scene_names": [scene_name]}):
        file_name = _generate_file_name()
        actual_path = tmp_path.with_name(file_name)
        with open(actual_path, "w") as outfile:
            outfile.write("")
            assert actual_path.exists()
            assert actual_path.is_file()


Review comment (suggestion):
The scene name must be a valid Python identifier, so this regex here can be sharpened a little:

```suggestion
    expected_pattern = r"[0-9a-zA-Z_]+[@_-]\d\d\d\d-\d\d-\d\d[@_-]\d\d-\d\d-\d\d"
```

Code after:
import re

from manim import tempconfig
from manim.utils.ipython_magic import _generate_file_name


def test_jupyter_file_naming():
    """Check the format of file names for jupyter"""
    scene_name = "SimpleScene"
    expected_pattern = r"[0-9a-zA-Z_]+[@_-]\d\d\d\d-\d\d-\d\d[@_-]\d\d-\d\d-\d\d"
    with tempconfig({"scene_names": [scene_name]}):
        file_name = _generate_file_name()
        match = re.match(expected_pattern, file_name)
        assert scene_name in file_name, (
            "Expected file to contain " + scene_name + " but got " + file_name
        )
        assert match, "file name does not match expected pattern " + expected_pattern


def test_jupyter_file_output(tmp_path):
    """Check the jupyter file naming is valid and can be created"""
    scene_name = "SimpleScene"
    with tempconfig({"scene_names": [scene_name]}):
        file_name = _generate_file_name()
        actual_path = tmp_path.with_name(file_name)
        with open(actual_path, "w") as outfile:
            outfile.write("")
            assert actual_path.exists()
            assert actual_path.is_file()
Repository: ManimCommunity/manim
File: tests/test_number_line.py (Python)

Code before:
import numpy as np

from manim import NumberLine


def test_unit_vector():
    """Check if the magnitude of unit vector along
    the NumberLine is equal to its unit_size."""
    axis1 = NumberLine(unit_size=0.4)
    axis2 = NumberLine(x_range=[-2, 5, 1])
    axis2.width = 12
    for axis in (axis1, axis2):
        assert np.linalg.norm(axis.get_unit_vector()) == axis.unit_size


Review comment (suggestion):
I'm not sure if this was intentional, but `NumberLine`'s length can be adjusted via the `length` parameter
```suggestion
    axis2 = NumberLine(x_range=[-2, 5, 1], length = 12)
```

Code after:
import numpy as np

from manim import NumberLine


def test_unit_vector():
    """Check if the magnitude of unit vector along
    the NumberLine is equal to its unit_size."""
    axis1 = NumberLine(unit_size=0.4)
    axis2 = NumberLine(x_range=[-2, 5], length=12)
    for axis in (axis1, axis2):
        assert np.linalg.norm(axis.get_unit_vector()) == axis.unit_size


def test_decimal_determined_by_step():
    """Checks that step size is considered when determining the number of decimal
    places."""
    axis = NumberLine(x_range=[-2, 2, 0.5])
    expected_decimal_places = 1
    actual_decimal_places = axis.decimal_number_config["num_decimal_places"]
    assert actual_decimal_places == expected_decimal_places, (
        "Expected 1 decimal place but got " + actual_decimal_places
    )

    axis2 = NumberLine(x_range=[-1, 1, 0.25])
    expected_decimal_places = 2
    actual_decimal_places = axis2.decimal_number_config["num_decimal_places"]
    assert actual_decimal_places == expected_decimal_places, (
        "Expected 1 decimal place but got " + actual_decimal_places
    )


def test_decimal_config_overrides_defaults():
    """Checks that ``num_decimal_places`` is determined by step size and gets overridden by ``decimal_number_config``."""
    axis = NumberLine(
        x_range=[-2, 2, 0.5], decimal_number_config={"num_decimal_places": 0}
Repository: ManimCommunity/manim
File: tests/test_number_line.py (Python)

Code before:
import numpy as np

from manim import NumberLine


def test_unit_vector():
    """Check if the magnitude of unit vector along
    the NumberLine is equal to its unit_size."""
    axis1 = NumberLine(unit_size=0.4)
    axis2 = NumberLine(x_range=[-2, 5, 1], length=12)
    for axis in (axis1, axis2):
        assert np.linalg.norm(axis.get_unit_vector()) == axis.unit_size


def test_decimal_determined_by_step():
    """Checks that step size is considered when determining the number of decimal
    places."""
    axis = NumberLine(x_range=[-2, 2, 0.5])
    expected_decimal_places = 1
    actual_decimal_places = axis.decimal_number_config["num_decimal_places"]
    assert actual_decimal_places == expected_decimal_places, (
        "Expected 1 decimal place but got " + actual_decimal_places
    )

    axis2 = NumberLine(x_range=[-1, 1, 0.25])
    expected_decimal_places = 2
    actual_decimal_places = axis2.decimal_number_config["num_decimal_places"]
    assert actual_decimal_places == expected_decimal_places, (
        "Expected 1 decimal place but got " + actual_decimal_places
    )


def test_decimal_config_overrides_defaults():
    """Checks that ``num_decimal_places`` is determined by step size and gets overridden by ``decimal_number_config``."""
    axis = NumberLine(

Review comment (suggestion):
Don't think you need to specify the step size
```suggestion
    axis2 = NumberLine(x_range=[-2, 5], length=12)
```

Code after:
import numpy as np

from manim import NumberLine


def test_unit_vector():
    """Check if the magnitude of unit vector along
    the NumberLine is equal to its unit_size."""
    axis1 = NumberLine(unit_size=0.4)
    axis2 = NumberLine(x_range=[-2, 5], length=12)
    for axis in (axis1, axis2):
        assert np.linalg.norm(axis.get_unit_vector()) == axis.unit_size


def test_decimal_determined_by_step():
    """Checks that step size is considered when determining the number of decimal
    places."""
    axis = NumberLine(x_range=[-2, 2, 0.5])
    expected_decimal_places = 1
    actual_decimal_places = axis.decimal_number_config["num_decimal_places"]
    assert actual_decimal_places == expected_decimal_places, (
        "Expected 1 decimal place but got " + actual_decimal_places
    )

    axis2 = NumberLine(x_range=[-1, 1, 0.25])
    expected_decimal_places = 2
    actual_decimal_places = axis2.decimal_number_config["num_decimal_places"]
    assert actual_decimal_places == expected_decimal_places, (
        "Expected 1 decimal place but got " + actual_decimal_places
    )


def test_decimal_config_overrides_defaults():
    """Checks that ``num_decimal_places`` is determined by step size and gets overridden by ``decimal_number_config``."""
    axis = NumberLine(
Repository: ManimCommunity/manim
File: tests/test_mobject.py (Python)

Code before:
import pytest

from manim import Mobject


def test_mobject_add():
    """Test Mobject.add()."""
    """Call this function with a Container instance to test its add() method."""
    # check that obj.submobjects is updated correctly
    obj = Mobject()
    assert len(obj.submobjects) == 0
    obj.add(Mobject())
    assert len(obj.submobjects) == 1
    obj.add(*(Mobject() for _ in range(10)))
    assert len(obj.submobjects) == 11

    # check that adding a mobject twice does not actually add it twice
    repeated = Mobject()
    obj.add(repeated)
    assert len(obj.submobjects) == 12
    obj.add(repeated)
    assert len(obj.submobjects) == 12

    # check that Container.add() returns the Mobject (for chained calls)
    assert obj.add(Mobject()) is obj
    obj = Mobject()

    # a Mobject cannot contain itself
    with pytest.raises(ValueError):
        obj.add(obj)

    # can only add Mobjects
    with pytest.raises(TypeError):
        obj.add("foo")

    assert obj.add(Mobject()) is obj


def test_mobject_remove():
    """Test Mobject.remove()."""
    obj = Mobject()
    to_remove = Mobject()
    obj.add(to_remove)
    obj.add(*(Mobject() for _ in range(10)))
    assert len(obj.submobjects) == 11
    obj.remove(to_remove)
    assert len(obj.submobjects) == 10
    obj.remove(to_remove)
    assert len(obj.submobjects) == 10

Review comment (suggestion):
```suggestion
    # Check that Mobject.add() returns the Mobject (for chained calls).
```

Code after:
import pytest

from manim import Mobject


def test_mobject_add():
    """Test Mobject.add()."""
    """Call this function with a Container instance to test its add() method."""
    # check that obj.submobjects is updated correctly
    obj = Mobject()
    assert len(obj.submobjects) == 0
    obj.add(Mobject())
    assert len(obj.submobjects) == 1
    obj.add(*(Mobject() for _ in range(10)))
    assert len(obj.submobjects) == 11

    # check that adding a mobject twice does not actually add it twice
    repeated = Mobject()
    obj.add(repeated)
    assert len(obj.submobjects) == 12
    obj.add(repeated)
    assert len(obj.submobjects) == 12

    # check that Mobject.add() returns the Mobject (for chained calls)
    assert obj.add(Mobject()) is obj
    obj = Mobject()

    # a Mobject cannot contain itself
    with pytest.raises(ValueError):
        obj.add(obj)

    # can only add Mobjects
    with pytest.raises(TypeError):
        obj.add("foo")


def test_mobject_remove():
    """Test Mobject.remove()."""
    obj = Mobject()
    to_remove = Mobject()
    obj.add(to_remove)
    obj.add(*(Mobject() for _ in range(10)))
    assert len(obj.submobjects) == 11
    obj.remove(to_remove)
    assert len(obj.submobjects) == 10
    obj.remove(to_remove)
    assert len(obj.submobjects) == 10

    assert obj.remove(Mobject()) is obj
Repository: ManimCommunity/manim
File: tests/test_scene.py (Python)

Code before:
import pytest

from manim import Mobject, Scene, tempconfig


def test_scene_add_remove():
    with tempconfig({"dry_run": True}):
        scene = Scene()
        assert len(scene.mobjects) == 0
        scene.add(Mobject())
        assert len(scene.mobjects) == 1
        scene.add(*(Mobject() for _ in range(10)))
        assert len(scene.mobjects) == 11

        # check that adding a msceneect twice does not actually add it twice
        repeated = Mobject()
        scene.add(repeated)
        assert len(scene.mobjects) == 12
        scene.add(repeated)
        assert len(scene.mobjects) == 12

        # check that Container.add() returns the Msceneect (for chained calls)
        assert scene.add(Mobject()) is scene
        to_remove = Mobject()
        scene = Scene()
        scene.add(to_remove)
        scene.add(*(Mobject() for _ in range(10)))
        assert len(scene.mobjects) == 11
        scene.remove(to_remove)
        assert len(scene.mobjects) == 10
        scene.remove(to_remove)
        assert len(scene.mobjects) == 10

        # check that Container.remove() returns the instance (for chained calls)
        assert scene.add(Mobject()) is scene


Review comment (suggestion):
```suggestion
        # Check that adding a mobject twice does not actually add it twice.
```

Code after:
import pytest

from manim import Mobject, Scene, tempconfig


def test_scene_add_remove():
    with tempconfig({"dry_run": True}):
        scene = Scene()
        assert len(scene.mobjects) == 0
        scene.add(Mobject())
        assert len(scene.mobjects) == 1
        scene.add(*(Mobject() for _ in range(10)))
        assert len(scene.mobjects) == 11

        # Check that adding a mobject twice does not actually add it twice
        repeated = Mobject()
        scene.add(repeated)
        assert len(scene.mobjects) == 12
        scene.add(repeated)
        assert len(scene.mobjects) == 12

        # Check that Scene.add() returns the Scene (for chained calls)
        assert scene.add(Mobject()) is scene
        to_remove = Mobject()
        scene = Scene()
        scene.add(to_remove)
        scene.add(*(Mobject() for _ in range(10)))
        assert len(scene.mobjects) == 11
        scene.remove(to_remove)
        assert len(scene.mobjects) == 10
        scene.remove(to_remove)
        assert len(scene.mobjects) == 10

        # Check that Scene.remove() returns the instance (for chained calls)
        assert scene.add(Mobject()) is scene