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

Code before:
        for i, (bar, value) in enumerate(zip(self.bars, values)):
            chart_val = self.values[i]

            if chart_val > 0:
                bar_lim = bar.get_bottom()
                aligned_edge = DOWN
            else:
                bar_lim = bar.get_top()
                aligned_edge = UP

            try:
                quotient = value / chart_val
                if quotient < 0:

                    aligned_edge = UP if chart_val > 0 else DOWN

                    # if the bar is already positive, then we now want to move it
                    # so that it is negative. So, we move the top edge of the bar
                    # to the location of the previous bottom

                    # if already negative, then we move the bottom edge of the bar
                    # to the location of the previous top

                bar.stretch_to_fit_height(abs(quotient) * bar.height)

            except ZeroDivisionError:
                # create a new bar since the current one has a height of zero (doesn't exist)
                temp_bar = self._create_bar(i, value)
                self.bars.remove(bar)
                self.bars.insert(i, temp_bar)

            bar.move_to(bar_lim, aligned_edge)

        if update_colors:
            self._update_colors()

        self.values[: len(values)] = values


Review comment (suggestion):
Can we change this to not use try/except. I mean we know when theres a division by zero error. No need to pull up a whole try/except. if else should do the job 🤷🏼 

Code after:
                    self.add(chart.get_bar_labels(font_size=24))
        """

        for i, (bar, value) in enumerate(zip(self.bars, values)):
            chart_val = self.values[i]

            if chart_val > 0:
                bar_lim = bar.get_bottom()
                aligned_edge = DOWN
            else:
                bar_lim = bar.get_top()
                aligned_edge = UP

            # check if the bar has height
            if chart_val != 0:
                quotient = value / chart_val
                if quotient < 0:

                    aligned_edge = UP if chart_val > 0 else DOWN

                    # if the bar is already positive, then we now want to move it
                    # so that it is negative. So, we move the top edge of the bar
                    # to the location of the previous bottom

                    # if already negative, then we move the bottom edge of the bar
                    # to the location of the previous top

                bar.stretch_to_fit_height(abs(quotient) * bar.height)

            else:
                # create a new bar since the current one has a height of zero (doesn't exist)
                temp_bar = self._create_bar(i, value)
                self.bars.remove(bar)
                self.bars.insert(i, temp_bar)

            bar.move_to(bar_lim, aligned_edge)

        if update_colors:
            self._update_colors()

        self.values[: len(values)] = values
Repository: ManimCommunity/manim
File: manim/utils/simple_functions.py (Python)

Code before:
    elif a > max_a:
        return max_a
    return a


def get_parameters(function: Callable) -> MappingProxyType[str, inspect.Parameter]:
    """Return the parameters of ``function`` as an ordered mapping of parameters'
    names to their corresponding ``Parameter`` objects.

    Examples
    --------
    ::

        >>> get_parameters(get_parameters)
        mappingproxy(OrderedDict([('function', <Parameter "function: 'Callable'">)]))

        >>> tuple(get_parameters(choose))
        ('n', 'k')
    """
    return inspect.signature(function).parameters


def sigmoid(x: float) -> float:
    r"""Returns the output of the logistic function.
    
    The logistic function, or sigmoid function, is defined
    as :math:`\frac{1}{1 + e^{-x}}`.

    References
    ----------
    - https://en.wikipedia.org/wiki/Sigmoid_function
    - https://en.wikipedia.org/wiki/Logistic_function
    """
    return 1.0 / (1 + np.exp(-x))


Review comment (suggestion):
```suggestion
    The logistic function, a common example of a sigmoid function, is defined
```
A minor [technicality on terminology](https://en.wikipedia.org/wiki/Sigmoid_function#:~:text=A%20common%20example%20of%20a%20sigmoid%20function%20is%20the%20logistic%20function) if it matters.

Code after:
    elif a > max_a:
        return max_a
    return a


def get_parameters(function: Callable) -> MappingProxyType[str, inspect.Parameter]:
    """Return the parameters of ``function`` as an ordered mapping of parameters'
    names to their corresponding ``Parameter`` objects.

    Examples
    --------
    ::

        >>> get_parameters(get_parameters)
        mappingproxy(OrderedDict([('function', <Parameter "function: 'Callable'">)]))

        >>> tuple(get_parameters(choose))
        ('n', 'k')
    """
    return inspect.signature(function).parameters


def sigmoid(x: float) -> float:
    r"""Returns the output of the logistic function.

    The logistic function, a common example of a sigmoid function, is defined
    as :math:`\frac{1}{1 + e^{-x}}`.

    References
    ----------
    - https://en.wikipedia.org/wiki/Sigmoid_function
    - https://en.wikipedia.org/wiki/Logistic_function
    """
    return 1.0 / (1 + np.exp(-x))
Repository: ManimCommunity/manim
File: manim/mobject/geometry/line.py (Python)

Code before:
        """

        if degrees:
            return self.angle_value / DEGREES
        return self.angle_value

    @staticmethod
    def from_three_points(
        A: np.array(), B: np.array(), C: np.array(), **kwargs
    ) -> Angle:
        """Create an instance of the :class:`Angle` class from three points.

        Parameters
        ----------
        A
            :class:Numpy.array()  -  End point of line 1
        B
            :class:Numpy.array()  -  Start point of lines 1 and 2
        C
            :class:Numpy.array()  -  End point of line 2
        **kwargs
            keyword arguments are passed to Angle constructor

        Returns
        -------
        : The Angle calculated from the three points

        Examples
        --------
        ::
            >>> A = np.array(UP)
            >>> B = np.array(ORIGIN)
            >>> C = np.array(RIGHT)
            >>> ang = Angle.from_three_points(A, B, C)
        """
        return Angle(Line(B, A), Line(B, C), **kwargs)


class RightAngle(Angle):
    """An elbow-type mobject representing a right angle between two lines.

    Parameters
    ----------
    line1
        The first line.
    line2
        The second line.
    length
        The length of the arms.
    **kwargs
        Further keyword arguments that are passed to the constructor of :class:`Angle`.

Review comment (suggestion):
```suggestion
        The Angle calculated from the three points
```

This might need to be indented correctly, we'll see how it renders.

Code after:
        """

        if degrees:
            return self.angle_value / DEGREES
        return self.angle_value

    @staticmethod
    def from_three_points(
        A: np.ndarray, B: np.ndarray, C: np.ndarray, **kwargs
    ) -> Angle:
        """The angle between the lines AB and BC.

        This constructs the angle :math:`\angle ABC`.

        Parameters
        ----------
        A
            The endpoint of the first angle leg
        B
            The vertex of the angle
        C
            The endpoint of the second angle leg

        **kwargs
            Further keyword arguments are passed to :class:`.Angle`

        Returns
        -------
        The Angle calculated from the three points

                    Angle(line1, line2, radius=0.5, quadrant=(-1,1), stroke_width=8),
                    Angle(line1, line2, radius=0.7, quadrant=(-1,-1), color=RED, other_angle=True),

        Examples
        --------
        .. manim:: AngleFromThreePointsExample
            :save_last_frame:

            class AngleFromThreePointsExample(Scene):
                def construct(self):
                    sample_angle = Angle.from_three_points(UP, ORIGIN, LEFT)
                    red_angle = Angle.from_three_points(LEFT + UP, ORIGIN, RIGHT, radius=.8, quadrant=(-1,-1), color=RED, stroke_width=8, other_angle=True)
                    self.add(red_angle, sample_angle)
        """
        return Angle(Line(B, A), Line(B, C), **kwargs)


class RightAngle(Angle):
    """An elbow-type mobject representing a right angle between two lines.

    Parameters
Repository: ManimCommunity/manim
File: tests/helpers/graphical_units.py (Python)

Code before:
"""Helpers functions for devs to set up new graphical-units data."""


from __future__ import annotations

import os
import tempfile
from operator import truediv
from pathlib import Path

import numpy as np

from manim import config, logger


def set_test_scene(scene_object, module_name):
    """Function used to set up the test data for a new feature. This will basically set up a pre-rendered frame for a scene. This is meant to be used only
    when setting up tests. Please refer to the wiki.

    Parameters
    ----------
    scene_object : :class:`~.Scene`
        The scene with which we want to set up a new test.
    module_name : :class:`str`
        The name of the module in which the functionality tested is contained. For example, ``Write`` is contained in the module ``creation``. This will be used in the folder architecture
        of ``/tests_data``.

    Examples
    --------
    Normal usage::
        set_test_scene(DotTest, "geometry")

    """

Review comment (suggestion):
I think this does not need to be added here?
```suggestion
```

Code after:
"""Helpers functions for devs to set up new graphical-units data."""


from __future__ import annotations

import tempfile
from pathlib import Path

import numpy as np

from manim import config, logger


def set_test_scene(scene_object, module_name):
    """Function used to set up the test data for a new feature. This will basically set up a pre-rendered frame for a scene. This is meant to be used only
    when setting up tests. Please refer to the wiki.

    Parameters
    ----------
    scene_object : :class:`~.Scene`
        The scene with which we want to set up a new test.
    module_name : :class:`str`
        The name of the module in which the functionality tested is contained. For example, ``Write`` is contained in the module ``creation``. This will be used in the folder architecture
        of ``/tests_data``.

    Examples
    --------
    Normal usage::
        set_test_scene(DotTest, "geometry")

    """
    config["write_to_movie"] = False
    config["disable_caching"] = True
Repository: ManimCommunity/manim
File: tests/helpers/video_utils.py (Python)

Code before:
"""Helpers for dev to set up new tests that use videos."""

from __future__ import annotations

import json
from pathlib import Path
from typing import Any

from manim import get_dir_layout, get_video_metadata, logger


def get_section_dir_layout(dirpath: str) -> list[str]:
    """Return a list of all files in the sections directory."""
    # test if sections have been created in the first place, doesn't work with multiple scene but this isn't an issue with tests
    if not Path(dirpath).is_dir():
        return []
    files = get_dir_layout(dirpath)
    # indicate that the sections directory has been created
    files.append(".")
    return files


def get_section_index(metapath: Path) -> list[dict[str, Any]]:
    """Return content of sections index file."""
    parent_folder = metapath.parent.absolute()
    # test if sections have been created in the first place
    if not parent_folder.is_dir():
        return []
    with metapath.open() as file:
        index = json.load(file)
    return index


def save_control_data_from_video(path_to_video: str, name: str) -> None:
    """Helper used to set up a new test that will compare videos.

    This will create a new ``.json`` file in ``control_data/videos_data`` that contains:

Review comment (suggestion):
Can you make it so that this function accepts a `Path` object?
And then not have a cast to string at line 64.

Code after:
"""Helpers for dev to set up new tests that use videos."""

from __future__ import annotations

import json
from pathlib import Path
from typing import Any

from manim import get_dir_layout, get_video_metadata, logger


def get_section_dir_layout(dirpath: Path) -> list[str]:
    """Return a list of all files in the sections directory."""
    # test if sections have been created in the first place, doesn't work with multiple scene but this isn't an issue with tests
    if not dirpath.is_dir():
        return []
    files = list(get_dir_layout(dirpath))
    # indicate that the sections directory has been created
    files.append(".")
    return files


def get_section_index(metapath: Path) -> list[dict[str, Any]]:
    """Return content of sections index file."""
    parent_folder = metapath.parent.absolute()
    # test if sections have been created in the first place
    if not parent_folder.is_dir():
        return []
    with metapath.open() as file:
        index = json.load(file)
    return index


def save_control_data_from_video(path_to_video: str, name: str) -> None:
    """Helper used to set up a new test that will compare videos.

    This will create a new ``.json`` file in ``control_data/videos_data`` that contains:
Repository: ManimCommunity/manim
File: manim/cli/cfg/group.py (Python)

Code before:


@cfg.command(context_settings=CONTEXT_SETTINGS)
@click.option("-d", "--directory", default=Path.cwd())
@click.pass_context
def export(ctx, directory):
    if Path(directory).absolute == Path(Path.cwd()).absolute:
        console.print(
            """You are reading the config from the same directory you are exporting to.
This means that the exported config will overwrite the config for this directory.
Are you sure you want to continue? (y/n)""",
            style="red bold",
            end="",
        )
        proceed = input().lower() == "y"
    else:
        proceed = True
    if proceed:
        if not Path(directory).is_dir():
            console.print(f"Creating folder: {directory}.", style="red bold")
            Path(directory).mkdir(parents=True)
        with open(
            Path(directory).joinpath("manim.cfg"), "w"
        ) as outpath:  # this is not used?
            ctx.invoke(write)
            from_path = Path(Path.cwd()).joinpath("manim.cfg")
            to_path = Path(directory).joinpath("manim.cfg")
        console.print(f"Exported final Config at {from_path} to {to_path}.")
    else:
        console.print("Aborted...", style="red bold")


Review comment (suggestion):
```suggestion
            from_path = Path.cwd() / "manim.cfg"
```
Path.cwd() returns a Path object so casting is not required also you can use the `/` operator.

Code after:
        console.print("\n")


@cfg.command(context_settings=cli_ctx_settings)
@click.option("-d", "--directory", default=Path.cwd())
@click.pass_context
def export(ctx, directory):
    directory_path = Path(directory)
    if directory_path.absolute == Path.cwd().absolute:
        console.print(
            """You are reading the config from the same directory you are exporting to.
This means that the exported config will overwrite the config for this directory.
Are you sure you want to continue? (y/n)""",
            style="red bold",
            end="",
        )
        proceed = input().lower() == "y"
    else:
        proceed = True
    if proceed:
        if not directory_path.is_dir():
            console.print(f"Creating folder: {directory}.", style="red bold")
            directory_path.mkdir(parents=True)

        ctx.invoke(write)
        from_path = Path.cwd() / "manim.cfg"
        to_path = directory_path / "manim.cfg"

        console.print(f"Exported final Config at {from_path} to {to_path}.")
    else:
        console.print("Aborted...", style="red bold")
Repository: ManimCommunity/manim
File: manim/animation/speedmodifier.py (Python)

Code before:
from __future__ import annotations

import types
from typing import Callable

from numpy import piecewise

from manim.utils.simple_functions import get_parameters

from ..animation.animation import Animation, Wait, prepare_animation
from ..animation.composition import AnimationGroup
from ..mobject.mobject import Mobject, Updater, _AnimationBuilder
from ..scene.scene import Scene


class ChangeSpeed(Animation):
    """
    Modifies the speed of passed animation. :class:`AnimationGroup` with
    different ``lag_ratio`` can also be used which combines multiple
    animations into one. `run_time` of the passed animation is changed to
    modify the speed.

    Parameters
    ----------
    anim : :class:`Animation` | :class:`_AnimationBuilder`
        Animation of which the speed is to be modified.
    speedinfo : Dict[float, float]
        Contains nodes (percentage of run_time) and its corresponding speed factor.
    rate_func : Callable[[float], float]
        Overrides `rate_func` of passed animation, applied before changing speed.

    Examples
    --------

    .. manim::SpeedModiferExample

        class SpeedModifierExample(Scene):
            def construct(self):
                a = Dot().shift(LEFT * 4)
                b = Dot().shift(RIGHT * 4)
                self.add(a, b)
                self.play(
                    ChangeSpeed(

Review comment (suggestion):
Just to be in line with our general docstring format.

```suggestion
    """Modifies the speed of passed animation. :class:`AnimationGroup` with
```

Code after:
"""Utilities for modifying the speed at which animations are played."""

from __future__ import annotations

import types
from typing import Callable

from numpy import piecewise

from manim.utils.simple_functions import get_parameters

from ..animation.animation import Animation, Wait, prepare_animation
from ..animation.composition import AnimationGroup
from ..mobject.mobject import Mobject, Updater, _AnimationBuilder
from ..scene.scene import Scene


class ChangeSpeed(Animation):
    """Modifies the speed of passed animation.
    :class:`AnimationGroup` with different ``lag_ratio`` can also be used
    which combines multiple animations into one.
    The ``run_time`` of the passed animation is changed to modify the speed.

    Parameters
    ----------
    anim
        Animation of which the speed is to be modified.
    speedinfo
        Contains nodes (percentage of ``run_time``) and its corresponding speed factor.
    rate_func
        Overrides ``rate_func`` of passed animation, applied before changing speed.

    Examples
    --------

    .. manim:: SpeedModifierExample

        class SpeedModifierExample(Scene):
            def construct(self):
                a = Dot().shift(LEFT * 4)
                b = Dot().shift(RIGHT * 4)
                self.add(a, b)
                self.play(
Repository: ManimCommunity/manim
File: manim/utils/bezier.py (Python)

Code before:

# Shortened version of partial_bezier_points just for quadratics,
# since this is called a fair amount
def partial_quadratic_bezier_points(points, a, b):
    if a == 1:
        return 3 * [points[-1]]

    def curve(t):
        return (
            points[0] * (1 - t) * (1 - t)
            + 2 * points[1] * t * (1 - t)
            + points[2] * t * t
        )

    # bezier(points)
    h0 = curve(a) if a > 0 else points[0]
    h2 = curve(b) if b < 1 else points[2]
    h1_prime = (1 - a) * points[1] + a * points[2]
    end_prop = (b - a) / (1.0 - a)
    h1 = (1 - end_prop) * h0 + end_prop * h1_prime
    return [h0, h1, h2]


def split_quadratic_bezier(points: Iterable[float], t: float) -> np.ndarray:
    """
    Split a quadratic bezier curve into two quadratic beziers at t

    Parameters
    ----------
    points : np.array
        The control points of the bezier curve
        has shape [a1, h1, b1]

    t : float
        The t-value at which to split the bezier curve

    Returns
    -------
        returns the two beziers as a list of tuples
        has the shape [a1, h1, b1], [a2, h2, b2]
    """
    a1, h1, a2 = points
    s1 = interpolate(a1, h1, t)
    s2 = interpolate(h1, a2, t)
    p = interpolate(s1, s2, t)

    return np.array([a1, s1, p, p, s2, a2])


def subdivide_quadratic_bezier(points: Iterable[float], n: int) -> np.ndarray:
    """Subdivide a quadratic bezier n times

Review comment (suggestion):
```suggestion
    """Split a quadratic Bézier curve at argument ``t`` into two quadratic curves.
```

Code after:

# Shortened version of partial_bezier_points just for quadratics,
# since this is called a fair amount
def partial_quadratic_bezier_points(points, a, b):
    if a == 1:
        return 3 * [points[-1]]

    def curve(t):
        return (
            points[0] * (1 - t) * (1 - t)
            + 2 * points[1] * t * (1 - t)
            + points[2] * t * t
        )

    # bezier(points)
    h0 = curve(a) if a > 0 else points[0]
    h2 = curve(b) if b < 1 else points[2]
    h1_prime = (1 - a) * points[1] + a * points[2]
    end_prop = (b - a) / (1.0 - a)
    h1 = (1 - end_prop) * h0 + end_prop * h1_prime
    return [h0, h1, h2]


def split_quadratic_bezier(points: Iterable[float], t: float) -> np.ndarray:
    """Split a quadratic Bézier curve at argument ``t`` into two quadratic curves.

    Parameters
    ----------
    points
        The control points of the bezier curve
        has shape ``[a1, h1, b1]``

    t
        The ``t``-value at which to split the Bézier curve

    Returns
    -------
        The two Bézier curves as a list of tuples,
        has the shape ``[a1, h1, b1], [a2, h2, b2]``
    """
    a1, h1, a2 = points
    s1 = interpolate(a1, h1, t)
    s2 = interpolate(h1, a2, t)
    p = interpolate(s1, s2, t)

    return np.array([a1, s1, p, p, s2, a2])


def subdivide_quadratic_bezier(points: Iterable[float], n: int) -> np.ndarray:
    """Subdivide a quadratic Bézier curve into ``n`` subcurves which have the same shape.