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

Bhavay-2001 · 2024-07-20T12:04:19Z

Description

Fix of issue #1384

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

I will update with colab notebook once the changes are approved.

Any specific deployment considerations

Docs

Docs updated? What were the changes:

yeldarby · 2024-07-20T15:15:42Z

Nice! Probably also needs custom_text_color_lookup override

Merge branch 'text_color_param' of https://github.com/Bhavay-2001/roboflow-supervision into text_color_param

…oflow-supervision into text_color_param

Bhavay-2001 · 2024-07-22T09:14:17Z

Hi @SkalskiP, I have made the changes and this is the colab. Please let me know if anything is wrong and anything needs to be changed.

I have only changed the Label Annotator for now and will change the other two based if this is correct.

Bhavay-2001 · 2024-07-23T08:43:26Z

Hi @SkalskiP, pls review this one.
Thanks

SkalskiP · 2024-07-23T09:34:11Z

supervision/annotators/core.py

@@ -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,


Let's use the same custom_color_lookup.

SkalskiP · 2024-07-23T09:43:50Z

Hi 👋🏻 @yeldarby and @Bhavay-2001. I don't think we need custom_text_color_lookup.

The spirit behind custom_color_lookup is to override color_lookup while maintaining its consistent behavior. In the cases of ColorLookup.INDEX, ColorLookup.CLASS, or ColorLookup.TRACK, we utilize a single integer value to select a color with the same index from the color and text_color palettes. I believe the same approach should be adopted here. We should utilize custom_color_lookup to pick a color with the same index from the color and text_color palettes.

@Bhavay-2001 please apply these same changes to RichLabelAnnotator 🙏🏻

Bhavay-2001 · 2024-07-24T07:55:42Z

supervision/annotators/core.py

@@ -1369,7 +1382,7 @@ def annotate(
                xy=(text_x, text_y),
                text=text,
                font=self.font,
-                fill=self.text_color.as_rgb(),
+                fill=text_color.as_rgb(),


In this, do we need to change as_rgb() to as_bgr() same like we did in LabelAnnotator?

RichLabelAnnotator uses Pillow as a rendering engine. Pillow accepts colors in RGB order, while OpenCV in BGR. So we keep as_rgb() here.

Bhavay-2001 · 2024-07-24T07:55:53Z

Hi @SkalskiP, please review the changes.

Bhavay-2001 · 2024-07-24T07:58:16Z

Also, if this gets approved, can I start with #1383? I would like to know more about the issue.
Thanks

onuralpszr · 2024-07-24T14:29:22Z

supervision/annotators/core.py

@@ -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,


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.

@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

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

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

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

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

…oflow-supervision into text_color_param

Bhavay-2001 · 2024-07-25T08:40:46Z

Hi @SkalskiP, this is open for reviewing now.

SkalskiP · 2024-07-25T13:35:42Z

supervision/annotators/core.py

@@ -1369,7 +1382,7 @@ def annotate(
                xy=(text_x, text_y),
                text=text,
                font=self.font,
-                fill=self.text_color.as_rgb(),
+                fill=text_color.as_rgb(),


RichLabelAnnotator uses Pillow as a rendering engine. Pillow accepts colors in RGB order, while OpenCV in BGR. So we keep as_rgb() here.

SkalskiP · 2024-07-25T13:36:58Z

supervision/annotators/core.py

        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,


Let's remove this parameter and use color_lookup.

SkalskiP · 2024-07-25T13:37:28Z

supervision/annotators/core.py

        font_path: Optional[str] = None,
        font_size: int = 10,
        text_padding: int = 10,
        text_position: Position = Position.TOP_LEFT,
        color_lookup: ColorLookup = ColorLookup.CLASS,
+        text_color_lookup: ColorLookup = ColorLookup.CLASS,


Let's remove this parameter and use only color_lookup.

SkalskiP · 2024-07-25T13:38:12Z

@Bhavay-2001, thanks a lot for all the work. We are almost there. Just few small changes and we can merge.

Bhavay-2001 · 2024-07-25T16:36:02Z

@Bhavay-2001, thanks a lot for all the work. We are almost there. Just few small changes and we can merge.

Done

SkalskiP · 2024-07-26T07:27:36Z

Tested here everything works. Merging!

Bhavay-2001 and others added 3 commits July 20, 2024 17:30

Updated the text_color parameter

60d33ca

Reduced length of the line

073500b

fix(pre_commit): 🎨 auto format pre-commit hooks

0210f51

Bhavay-2001 and others added 6 commits July 22, 2024 13:06

Added custom text color lookup

16b8102

:wq

3fb6a7b

Merge branch 'text_color_param' of https://github.com/Bhavay-2001/roboflow-supervision into text_color_param

fix(pre_commit): 🎨 auto format pre-commit hooks

01922f8

Changed formatting

1f6694b

Merge branch 'text_color_param' of https://github.com/Bhavay-2001/rob…

e2f33fa

…oflow-supervision into text_color_param

text_color from DEFAULT to LEGACY

c5d052e

SkalskiP requested changes Jul 23, 2024

View reviewed changes

Bhavay-2001 and others added 2 commits July 24, 2024 13:24

Updated RichLabelAnnotator

a0abf2d

fix(pre_commit): 🎨 auto format pre-commit hooks

b8e1612

Bhavay-2001 commented Jul 24, 2024

View reviewed changes

onuralpszr reviewed Jul 24, 2024

View reviewed changes

Bhavay-2001 added 2 commits July 25, 2024 14:02

Added text_color param to RichLabelAnnotator

b103ce4

Merge branch 'text_color_param' of https://github.com/Bhavay-2001/rob…

9956d5d

…oflow-supervision into text_color_param

SkalskiP requested changes Jul 25, 2024

View reviewed changes

Removed text_color_lookup

37a988f

SkalskiP merged commit bb0e4ec into roboflow:develop Jul 26, 2024
9 checks passed

SkalskiP mentioned this pull request Jul 26, 2024

[LabelAnnotator, RichLabelAnnotator, VertexLabelAnnotator] - allow text_color to be Union[Color, ColorPalette] #1384

Closed

LinasKo mentioned this pull request Aug 14, 2024

Added text_color parameter for VertexLabelAnnotator #1409

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Bhavay-2001 commented Jul 20, 2024

yeldarby commented Jul 20, 2024

Bhavay-2001 commented Jul 22, 2024

Bhavay-2001 commented Jul 23, 2024

SkalskiP Jul 23, 2024

Bhavay-2001 Jul 23, 2024

SkalskiP commented Jul 23, 2024 •

edited

Loading

Bhavay-2001 Jul 24, 2024

SkalskiP Jul 25, 2024

Bhavay-2001 commented Jul 24, 2024

Bhavay-2001 commented Jul 24, 2024

onuralpszr Jul 24, 2024

Bhavay-2001 Jul 25, 2024

onuralpszr Jul 25, 2024 •

edited

Loading

Bhavay-2001 Jul 25, 2024

onuralpszr Jul 25, 2024

Bhavay-2001 commented Jul 25, 2024

SkalskiP Jul 25, 2024

SkalskiP Jul 25, 2024

SkalskiP Jul 25, 2024

SkalskiP commented Jul 25, 2024

Bhavay-2001 commented Jul 25, 2024

SkalskiP commented Jul 26, 2024

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

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

Conversation

Bhavay-2001 commented Jul 20, 2024

Description

Type of change

How has this change been tested, please provide a testcase or example of how you tested the change?

Any specific deployment considerations

Docs

yeldarby commented Jul 20, 2024

Bhavay-2001 commented Jul 22, 2024

Bhavay-2001 commented Jul 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SkalskiP commented Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bhavay-2001 commented Jul 24, 2024

Bhavay-2001 commented Jul 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onuralpszr Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bhavay-2001 commented Jul 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SkalskiP commented Jul 25, 2024

Bhavay-2001 commented Jul 25, 2024

SkalskiP commented Jul 26, 2024

SkalskiP commented Jul 23, 2024 •

edited

Loading

onuralpszr Jul 25, 2024 •

edited

Loading