ronantakizawa/codereview-bench huggingface dataset:
Repository: roboflow/supervision
File: supervision/keypoint/skeletons.py (Python)

Code before:


class Skeleton(Enum):
    COCO = [
        (1, 2),
        (1, 3),
        (2, 3),
        (2, 4),
        (3, 5),
        (6, 12),
        (6, 7),
        (6, 8),
        (7, 13),
        (7, 9),
        (8, 10),
        (9, 11),
        (12, 13),
        (14, 12),
        (15, 13),
        (16, 14),
        (17, 15),
    ]

    # Hardcoding here, but it also comes within model results
    # Note: the classes in the model are 0-indexed, while the skeleton is 1-indexed
    YOLO_NAS = [
        (0, 1),
        (0, 2),
        (1, 2),
        (1, 3),
        (2, 4),
        (3, 5),
        (4, 6),
        (5, 6),
        (5, 7),
        (5, 11),
        (6, 8),
        (6, 12),
        (7, 9),
        (8, 10),
        (11, 12),
        (11, 13),
        (12, 14),
        (13, 15),
        (14, 16),
    ]


SKELETONS_BY_EDGE_COUNT: Dict[int, Edges] = {}
SKELETONS_BY_VERTEX_COUNT: Dict[int, Edges] = {}


Review comment (suggestion):
Let's drop this skeleton. The only difference between this and the COCO skeleton we already have is that it has 2 more edges that connect the head to the torso. That's why we should aim to annotate YOLO NAS result in the same way as YOLOv8. 

Code after:


class Skeleton(Enum):
    COCO = [
        (1, 2),
        (1, 3),
        (2, 3),
        (2, 4),
        (3, 5),
        (6, 12),
        (6, 7),
        (6, 8),
        (7, 13),
        (7, 9),
        (8, 10),
        (9, 11),
        (12, 13),
        (14, 12),
        (15, 13),
        (16, 14),
        (17, 15),
    ]


SKELETONS_BY_EDGE_COUNT: Dict[int, Edges] = {}
SKELETONS_BY_VERTEX_COUNT: Dict[int, Edges] = {}

for skeleton in Skeleton:
    SKELETONS_BY_EDGE_COUNT[len(skeleton.value)] = skeleton.value

    unique_vertices = set(vertex for edge in skeleton.value for vertex in edge)
    SKELETONS_BY_VERTEX_COUNT[len(unique_vertices)] = skeleton.value
Repository: roboflow/supervision
File: supervision/detection/tools/inference_slicer.py (Python)

Code before:
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Callable, Optional, Tuple

import numpy as np

from supervision.detection.core import Detections
from supervision.detection.utils import move_boxes, move_masks
from supervision.utils.image import crop_image


def move_detections(
    detections: Detections, offset: np.ndarray, image_shape: np.ndarray
) -> Detections:
    """
    Args:
        detections (sv.Detections): Detections object to be moved.
        offset (np.ndarray): An array of shape `(2,)` containing offset values in format
            is `[dx, dy]`.
        image_size (np.ndarray): An array of shape `(2,)` or `(3,)`, size of the image
            is in format `[width, height]`.
    Returns:
        (sv.Detections) repositioned Detections object.
    """
    detections.xyxy = move_boxes(xyxy=detections.xyxy, offset=offset)
    if detections.mask is not None:
        detections.mask = move_masks(
            masks=detections.mask, offset=offset, desired_shape=image_shape
        )
    return detections


class InferenceSlicer:
    """
    InferenceSlicer performs slicing-based inference for small target detection. This
    method, often referred to as
    [Slicing Adaptive Inference (SAHI)](https://ieeexplore.ieee.org/document/9897990),
    involves dividing a larger image into smaller slices, performing inference on each

Review comment (suggestion):
Argument `image_shape` is called `resolution_wh` in most places in `supervision`. Let's keep it this way. 

Code after:
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Callable, Optional, Tuple

import numpy as np

from supervision.detection.core import Detections
from supervision.detection.utils import move_boxes, move_masks
from supervision.utils.image import crop_image


def move_detections(
    detections: Detections,
    offset: np.ndarray,
    resolution_wh: Optional[Tuple[int, int]] = None,
) -> Detections:
    """
    Args:
        detections (sv.Detections): Detections object to be moved.
        offset (np.ndarray): An array of shape `(2,)` containing offset values in format
            is `[dx, dy]`.
        resolution_wh (Tuple[int, int]): The width and height of the desired mask
            resolution. Required for segmentation detections.

    Returns:
        (sv.Detections) repositioned Detections object.
    """
    detections.xyxy = move_boxes(xyxy=detections.xyxy, offset=offset)
    if detections.mask is not None:
        if resolution_wh is None:
            raise ValueError(
                "Resolution width and height are required for moving segmentation "
                "detections. This should be the same as (width, height) of image shape."
            )
        detections.mask = move_masks(
            masks=detections.mask, offset=offset, resolution_wh=resolution_wh
        )
    return detections
Repository: roboflow/supervision
File: test/detection/test_line_counter.py (Python)

Code before:
                Point(0, 0),
                Point(0, 100),
            ),
            [
                [100, 50, 120, 70],
                [-100, 50, -80, 70],
            ],
        ),
        (
            Vector(
                Point(0, 0),
                Point(0, 100),
            ),
            [
                [-100, 50, -80, 70],
                [100, 50, 120, 70],
            ],
        ),
    ],
)
def test_line_zone_does_not_count_detections_without_tracker_id(vector, bbox_sequence):
    line_zone = LineZone(start=vector.start, end=vector.end)
    for bbox in bbox_sequence:
        detections = Detections(
            xyxy=np.array([bbox]).reshape((-1, 4)),
            tracker_id=np.array([None for _ in range(0, 1)]),
        )
        count_in, count_out = line_zone.trigger(detections)
        assert np.all(not count_in)
        assert np.all(not count_out)


Review comment (suggestion):
1. That's just [0]. No need for a for loop.
2. I don't think `None` can appear in `tracker_id`. But let's pretend it can, and I'll track it down in #1092
    * If you've seen `None` inside `tracker_id` when testing on your videos, do tell :slightly_smiling_face: 

Code after:
                [4, 4 - 10, 6, 6 - 10],
                [4, 4, 6, 6],
                [4, 4 - 10, 6, 6 - 10],
            ],
            [
                Position.CENTER,
                Position.CENTER_LEFT,
                Position.CENTER_RIGHT,
                Position.TOP_CENTER,
                Position.TOP_LEFT,
                Position.TOP_RIGHT,
                Position.BOTTOM_LEFT,
                Position.BOTTOM_CENTER,
                Position.BOTTOM_RIGHT,
            ],
            [False, True, False, True],
            [False, False, True, False],
        ),
    ],
)
def test_line_zone_one_detection(
    vector: Vector,
    xyxy_sequence: List[List[float]],
    triggering_anchors: List[Position],
    expected_crossed_in: List[bool],
    expected_crossed_out: List[bool],
) -> None:
    line_zone = LineZone(
        start=vector.start, end=vector.end, triggering_anchors=triggering_anchors
    )

    crossed_in_list = []
    crossed_out_list = []
    for i, bbox in enumerate(xyxy_sequence):
        detections = mock_detections(
            xyxy=[bbox],
            tracker_id=[0],
        )
        crossed_in, crossed_out = line_zone.trigger(detections)
        crossed_in_list.append(crossed_in[0])
        crossed_out_list.append(crossed_out[0])

    assert (
        crossed_in_list == expected_crossed_in
    ), f"expected {expected_crossed_in}, got {crossed_in_list}"
    assert (
        crossed_out_list == expected_crossed_out
    ), f"expected {expected_crossed_out}, got {crossed_out_list}"


@pytest.mark.parametrize(
Repository: roboflow/supervision
File: supervision/keypoint/core.py (Python)

Code before:
from __future__ import annotations

import math
from contextlib import suppress
from dataclasses import dataclass, field
from typing import Any, Dict, Iterator, List, Optional, Tuple, Union

import numpy as np
import numpy.typing as npt

from supervision.config import CLASS_NAME_DATA_FIELD
from supervision.detection.utils import get_data_item, is_data_equal
from supervision.validators import validate_keypoints_fields


@dataclass
class KeyPoints:
    """
    The `sv.KeyPoints` class in the Supervision library standardizes results from
    various keypoint detection and pose estimation models into a consistent format. This
    class simplifies data manipulation and filtering, providing a uniform API for
    integration with Supervision annotators.

    === "Ultralytics"

        Use [`sv.KeyPoints.from_ultralytics`](/keypoint/core/#supervision.keypoint.core.KeyPoints.from_ultralytics)
        method, which accepts model results.


Review comment (refactor):
Normally we use `numpy` rather than `math`. However, as per my later comment, we don't need any of them here.

Please remove this line :)

Code after:
from __future__ import annotations

from contextlib import suppress
from dataclasses import dataclass, field
from typing import Any, Dict, Iterator, List, Optional, Tuple, Union

import numpy as np
import numpy.typing as npt

from supervision.config import CLASS_NAME_DATA_FIELD
from supervision.detection.utils import get_data_item, is_data_equal
from supervision.validators import validate_keypoints_fields


@dataclass
class KeyPoints:
    """
    The `sv.KeyPoints` class in the Supervision library standardizes results from
    various keypoint detection and pose estimation models into a consistent format. This
    class simplifies data manipulation and filtering, providing a uniform API for
    integration with Supervision [keypoints annotators](/keypoint/annotators).

    === "Ultralytics"

        Use [`sv.KeyPoints.from_ultralytics`](/keypoint/core/#supervision.keypoint.core.KeyPoints.from_ultralytics)
        method, which accepts [YOLOv8](https://github.com/ultralytics/ultralytics)
        pose result.
Repository: roboflow/supervision
File: supervision/detection/tools/inference_slicer.py (Python)

Code before:
import warnings
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Callable, Optional, Tuple, Union

import numpy as np

from supervision.detection.core import Detections
from supervision.detection.overlap_filter import OverlapFilter, validate_overlap_filter
from supervision.detection.utils import move_boxes, move_masks, move_obb_boxes
from supervision.utils.image import crop_image
from supervision.utils.internal import SupervisionWarnings
from supervision import config


def move_detections(
    detections: Detections,
    offset: np.ndarray,
    resolution_wh: Optional[Tuple[int, int]] = None,
) -> Detections:
    """
    Args:
        detections (sv.Detections): Detections object to be moved.
        offset (np.ndarray): An array of shape `(2,)` containing offset values in format
            is `[dx, dy]`.
        resolution_wh (Tuple[int, int]): The width and height of the desired mask
            resolution. Required for segmentation detections.

    Returns:
        (sv.Detections) repositioned Detections object.
    """
    detections.xyxy = move_boxes(xyxy=detections.xyxy, offset=offset)
    if config.ORIENTED_BOX_COORDINATES in detections.data:
        detections.data[config.ORIENTED_BOX_COORDINATES] = move_obb_boxes(
            xyxyxyxy=detections.data[config.ORIENTED_BOX_COORDINATES], offset=offset
        )
    if detections.mask is not None:
        if resolution_wh is None:

Review comment (suggestion):
Let's only import `ORIENTED_BOX_COORDINATES`, rather than the whole config. This matches the pattern in the rest of the repo.

Code after:
import warnings
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Callable, Optional, Tuple, Union

import numpy as np

from supervision.config import ORIENTED_BOX_COORDINATES
from supervision.detection.core import Detections
from supervision.detection.overlap_filter import OverlapFilter, validate_overlap_filter
from supervision.detection.utils import move_boxes, move_masks, move_oriented_boxes
from supervision.utils.image import crop_image
from supervision.utils.internal import SupervisionWarnings


def move_detections(
    detections: Detections,
    offset: np.ndarray,
    resolution_wh: Optional[Tuple[int, int]] = None,
) -> Detections:
    """
    Args:
        detections (sv.Detections): Detections object to be moved.
        offset (np.ndarray): An array of shape `(2,)` containing offset values in format
            is `[dx, dy]`.
        resolution_wh (Tuple[int, int]): The width and height of the desired mask
            resolution. Required for segmentation detections.

    Returns:
        (sv.Detections) repositioned Detections object.
    """
    detections.xyxy = move_boxes(xyxy=detections.xyxy, offset=offset)
    if ORIENTED_BOX_COORDINATES in detections.data:
        detections.data[ORIENTED_BOX_COORDINATES] = move_oriented_boxes(
            xyxyxyxy=detections.data[ORIENTED_BOX_COORDINATES], offset=offset
        )
    if detections.mask is not None:
        if resolution_wh is None:
Repository: roboflow/supervision
File: supervision/detection/tools/inference_slicer.py (Python)

Code before:
import warnings
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Callable, Optional, Tuple, Union

import numpy as np

from supervision.config import ORIENTED_BOX_COORDINATES as o_b_c
from supervision.detection.core import Detections
from supervision.detection.overlap_filter import OverlapFilter, validate_overlap_filter
from supervision.detection.utils import move_boxes, move_masks, move_obb_boxes
from supervision.utils.image import crop_image
from supervision.utils.internal import SupervisionWarnings


def move_detections(
    detections: Detections,
    offset: np.ndarray,
    resolution_wh: Optional[Tuple[int, int]] = None,
) -> Detections:
    """
    Args:
        detections (sv.Detections): Detections object to be moved.
        offset (np.ndarray): An array of shape `(2,)` containing offset values in format
            is `[dx, dy]`.
        resolution_wh (Tuple[int, int]): The width and height of the desired mask
            resolution. Required for segmentation detections.

    Returns:
        (sv.Detections) repositioned Detections object.
    """
    detections.xyxy = move_boxes(xyxy=detections.xyxy, offset=offset)
    if o_b_c in detections.data:

Review comment (suggestion):
Please import `from supervision.config import ORIENTED_BOX_COORDINATES` and skip `as o_b_c`. The name is long but descriptive. It is also consistent with the rest of the codebase. 

Code after:
import warnings
from concurrent.futures import ThreadPoolExecutor, as_completed
from typing import Callable, Optional, Tuple, Union

import numpy as np

from supervision.config import ORIENTED_BOX_COORDINATES
from supervision.detection.core import Detections
from supervision.detection.overlap_filter import OverlapFilter, validate_overlap_filter
from supervision.detection.utils import move_boxes, move_masks, move_oriented_boxes
from supervision.utils.image import crop_image
from supervision.utils.internal import SupervisionWarnings


def move_detections(
    detections: Detections,
    offset: np.ndarray,
    resolution_wh: Optional[Tuple[int, int]] = None,
) -> Detections:
    """
    Args:
        detections (sv.Detections): Detections object to be moved.
        offset (np.ndarray): An array of shape `(2,)` containing offset values in format
            is `[dx, dy]`.
        resolution_wh (Tuple[int, int]): The width and height of the desired mask
            resolution. Required for segmentation detections.

    Returns:
        (sv.Detections) repositioned Detections object.
    """
    detections.xyxy = move_boxes(xyxy=detections.xyxy, offset=offset)
    if ORIENTED_BOX_COORDINATES in detections.data:
Repository: roboflow/supervision
File: supervision/annotators/core.py (Python)

Code before:
        Example:
            ```python
            import supervision as sv

            image = ...
            detections = sv.Detections(...)

            background_color_annotator = sv.BackgroundColorAnnotator()
            annotated_frame = background_color_annotator.annotate(
                scene=image.copy(),
                detections=detections
            )
            ```

        ![background-color-annotator-example](https://media.roboflow.com/
        supervision-annotator-examples/background-color-annotator-example-purple.png)
        """

        colored_mask = np.full_like(scene, self.color.as_bgr(), dtype=np.uint8)

        cv2.addWeighted(
            scene, 1 - self.opacity, colored_mask, self.opacity, 0, dst=colored_mask
        )

        if detections.mask is None or self.force_box:
            for detection_idx in range(len(detections)):
                x1, y1, x2, y2 = detections.xyxy[detection_idx].astype(int)
                colored_mask[y1:y2, x1:x2] = scene[y1:y2, x1:x2]
        else:
            for mask in detections.mask:
                colored_mask[mask] = scene[mask]

        return colored_mask


Review comment (suggestion):
`for x1, y1, x2, y2 in detections.xyxy.astype(int)` should work as well and be shorter. 

Code after:
                `ImageType` is a flexible type, accepting either `numpy.ndarray`
                or `PIL.Image.Image`.
            detections (Detections): Object detections to annotate.

        Returns:
            The annotated image, matching the type of `scene` (`numpy.ndarray`
                or `PIL.Image.Image`)

        Example:
            ```python
            import supervision as sv

            image = ...
            detections = sv.Detections(...)

            background_overlay_annotator = sv.BackgroundOverlayAnnotator()
            annotated_frame = background_overlay_annotator.annotate(
                scene=image.copy(),
                detections=detections
            )
            ```

        ![background-overlay-annotator-example](https://media.roboflow.com/
        supervision-annotator-examples/background-color-annotator-example-purple.png)
        """
        colored_mask = np.full_like(scene, self.color.as_bgr(), dtype=np.uint8)

        cv2.addWeighted(
            scene, 1 - self.opacity, colored_mask, self.opacity, 0, dst=colored_mask
        )

        if detections.mask is None or self.force_box:
            for x1, y1, x2, y2 in detections.xyxy.astype(int):
                colored_mask[y1:y2, x1:x2] = scene[y1:y2, x1:x2]
        else:
            for mask in detections.mask:
                colored_mask[mask] = scene[mask]

        np.copyto(scene, colored_mask)
        return scene
Repository: roboflow/supervision
File: supervision/keypoint/skeletons.py (Python)

Code before:
from enum import Enum
from typing import ClassVar, Dict, List, Tuple

Edges = List[Tuple[int, int]]


class Skeleton(Enum):
    COCO: ClassVar[List[Tuple[int, int]]] = [
        (1, 2),
        (1, 3),
        (2, 3),
        (2, 4),
        (3, 5),
        (6, 12),
        (6, 7),
        (6, 8),
        (7, 13),
        (7, 9),
        (8, 10),
        (9, 11),
        (12, 13),
        (14, 12),
        (15, 13),
        (16, 14),
        (17, 15),
    ]

    GHUM: ClassVar[List[Tuple[int, int]]] = [
        (1, 2),
        (1, 5),
        (2, 3),
        (3, 4),
        (4, 8),

Review comment (suggestion):
1. We can use `Edges` here.
2. Is `ClassVar` required here? Does it capture something about the contained data that would otherwise confuse MyPy or the user? In C++ terms - it feels more related to the `lvalue` than the `rvalue`. More about how the data is stored than what the data is.



Code after:
from enum import Enum
from typing import Dict, Tuple

Edges = Tuple[Tuple[int, int], ...]


class Skeleton(Enum):
    COCO: Edges = (
        (1, 2),
        (1, 3),
        (2, 3),
        (2, 4),
        (3, 5),
        (6, 12),
        (6, 7),
        (6, 8),
        (7, 13),
        (7, 9),
        (8, 10),
        (9, 11),
        (12, 13),
        (14, 12),
        (15, 13),
        (16, 14),
        (17, 15),
    )

    GHUM: Edges = (
        (1, 2),
        (1, 5),
        (2, 3),
        (3, 4),
        (4, 8),
Repository: roboflow/supervision
File: supervision/draw/utils.py (Python)

Code before:
import os
from typing import Optional, Tuple, Union

import cv2
import numpy as np

from supervision.draw.color import Color
from supervision.geometry.core import Point, Rect


def draw_line(
    scene: np.ndarray, start: Point, end: Point, color: Color=Color(163, 81, 251), thickness: int = 2
) -> np.ndarray:
    """
    Draws a line on a given scene.

    Parameters:
        scene (np.ndarray): The scene on which the line will be drawn
        start (Point): The starting point of the line
        end (Point): The end point of the line
        color (Color): The color of the line
        thickness (int): The thickness of the line

    Returns:
        np.ndarray: The scene with the line drawn on it
    """
    cv2.line(
        scene,
        start.as_xy_int_tuple(),
        end.as_xy_int_tuple(),
        color.as_bgr(),
        thickness=thickness,
    )
    return scene


def draw_rectangle(

Review comment (suggestion):
Please use our color scheme in like "sv.Color.WHITE"  for all them. You can find them in here 

https://github.com/roboflow/supervision/blob/337b0e4a5b5df9617ecd166a008b92472526bd2f/supervision/draw/color.py#L231

Code after:
import os
from typing import Optional, Tuple, Union

import cv2
import numpy as np

from supervision.draw.color import Color
from supervision.geometry.core import Point, Rect


def draw_line(
    scene: np.ndarray,
    start: Point,
    end: Point,
    color: Color = Color.ROBOFLOW,
    thickness: int = 2,
) -> np.ndarray:
    """
    Draws a line on a given scene.

    Parameters:
        scene (np.ndarray): The scene on which the line will be drawn
        start (Point): The starting point of the line
        end (Point): The end point of the line
        color (Color): The color of the line, defaults to Color.ROBOFLOW
        thickness (int): The thickness of the line

    Returns:
        np.ndarray: The scene with the line drawn on it
    """
    cv2.line(
        scene,
        start.as_xy_int_tuple(),
        end.as_xy_int_tuple(),
        color.as_bgr(),
        thickness=thickness,
    )