Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Union[Color, ColorPalette] to text_color parameter #1387

Merged
merged 14 commits into from
Jul 26, 2024
Merged
27 changes: 23 additions & 4 deletions supervision/annotators/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1011,37 +1011,42 @@ class LabelAnnotator(BaseAnnotator):
def __init__(
self,
color: Union[Color, ColorPalette] = ColorPalette.DEFAULT,
text_color: Color = Color.WHITE,
text_color: Union[Color, ColorPalette] = ColorPalette.LEGACY,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion: I think we should keep white as default because it changes readability and also change default behavior, up until this version we put "white" and when people see different color It can create some confusion. It would be better keep "Color.WHITE" as default but keep type change is "Union[Color, ColorPalette]" in case of multi color change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onuralpszr, yess certainly. I changed this because it was overlapping with ColorPalette.DEFAULT and the labels weren't visible but I will change it to White now.

Also, couldn't understand this but keep type change is "Union[Color, ColorPalette]" in case of multi color change. Can you explain it further? Thanks

Copy link
Collaborator

@onuralpszr onuralpszr Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bhavay-2001 What I meant is finally It should look like this

text_color: Union[Color, ColorPalette] = Color.WHITE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong on this but I think there is no such method. Although, there is COLOR.WHITE

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bhavay-2001 ah right I miss typed yes Color.WHITE

text_scale: float = 0.5,
text_thickness: int = 1,
text_padding: int = 10,
text_position: Position = Position.TOP_LEFT,
color_lookup: ColorLookup = ColorLookup.CLASS,
text_color_lookup: ColorLookup = ColorLookup.CLASS,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this parameter and use color_lookup.

border_radius: int = 0,
):
"""
Args:
color (Union[Color, ColorPalette]): The color or color palette to use for
annotating the text background.
text_color (Color): The color to use for the text.
text_color (Union[Color, ColorPalette]): The color or color palette to use
for the text.
text_scale (float): Font scale for the text.
text_thickness (int): Thickness of the text characters.
text_padding (int): Padding around the text within its background box.
text_position (Position): Position of the text relative to the detection.
Possible values are defined in the `Position` enum.
color_lookup (ColorLookup): Strategy for mapping colors to annotations.
Options are `INDEX`, `CLASS`, `TRACK`.
text_color_lookup (ColorLookup): Strategy for mapping text colors to
annotations. Options are `INDEX`, `CLASS`, `TRACK`.
border_radius (int): The radius to apply round edges. If the selected
value is higher than the lower dimension, width or height, is clipped.
"""
self.border_radius: int = border_radius
self.color: Union[Color, ColorPalette] = color
self.text_color: Color = text_color
self.text_color: Union[Color, ColorPalette] = text_color
self.text_scale: float = text_scale
self.text_thickness: int = text_thickness
self.text_padding: int = text_padding
self.text_anchor: Position = text_position
self.color_lookup: ColorLookup = color_lookup
self.text_color_lookup: ColorLookup = text_color_lookup

@ensure_cv2_image_for_annotation
def annotate(
Expand All @@ -1050,6 +1055,7 @@ def annotate(
detections: Detections,
labels: Optional[List[str]] = None,
custom_color_lookup: Optional[np.ndarray] = None,
custom_text_color_lookup: Optional[np.ndarray] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the same custom_color_lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

) -> ImageType:
"""
Annotates the given scene with labels based on the provided detections.
Expand All @@ -1062,6 +1068,8 @@ def annotate(
labels (Optional[List[str]]): Custom labels for each detection.
custom_color_lookup (Optional[np.ndarray]): Custom color lookup array.
Allows to override the default color mapping strategy.
custom_text_color_lookup (Optional[np.ndarray]): Custom text color lookup
array. Allows to override the default text color mapping strategy.

Returns:
The annotated image, matching the type of `scene` (`numpy.ndarray`
Expand Down Expand Up @@ -1114,6 +1122,17 @@ def annotate(
),
)

text_color = resolve_color(
color=self.text_color,
detections=detections,
detection_idx=detection_idx,
color_lookup=(
self.text_color_lookup
if custom_text_color_lookup is None
else custom_text_color_lookup
),
)

if labels is not None:
text = labels[detection_idx]
elif detections[CLASS_NAME_DATA_FIELD] is not None:
Expand Down Expand Up @@ -1152,7 +1171,7 @@ def annotate(
org=(text_x, text_y),
fontFace=font,
fontScale=self.text_scale,
color=self.text_color.as_bgr(),
color=text_color.as_bgr(),
thickness=self.text_thickness,
lineType=cv2.LINE_AA,
)
Expand Down