Fix annotation of draw_segmentation_masks #4527

oke-aditya · 2021-10-02T17:27:25Z

draw_segmentation_masks also supports passing single str containing color name or single str containing color hex or tuple containing RGB values as all colors for masks.

A simple example is our gallery 😃

https://pytorch.org/vision/master/auto_examples/plot_repurposing_annotations.html#sphx-glr-auto-examples-plot-repurposing-annotations-py

I have updated the tests too which all check all the above cases.

cc @NicolasHug

oke-aditya · 2021-10-02T19:40:59Z

I think that we should generate a color palette for draw_bounding_boxes too instead of drawing default white color.
If people really want white color boxes (yuk!) they can pass color ="white"

NicolasHug

Thanks @oke-aditya , this looks good, I made a few comments below

test/test_utils.py

NicolasHug · 2021-10-04T07:30:50Z

torchvision/utils.py

+        colors_.append(torch.tensor(color, dtype=out_dtype))

    img_to_draw = image.detach().clone()
    # TODO: There might be a way to vectorize this
-    for mask, color in zip(masks, colors_):
-        img_to_draw[:, mask] = color[:, None]
+    for mask, color_tensor in zip(masks, colors_):
+        img_to_draw[:, mask] = color_tensor[:, None]


In order to preserve git blame I would suggest to revert these changes, as I don't think they significantly improve readability either

Problem is mypy. To satisfy mypy I had to change the name.

what is mypy complaining about?

Mypy complains that color isn't a tensor, as we are re-using the name color.
Here is the exact error.

torchvision/utils.py:292: error: Incompatible types in assignment (expression has type "Tensor", variable has type "Union[str, Tuple[int, int, int]]") [assignment] color = torch.tensor(color, dtype=out_dtype) ^ torchvision/utils.py:298: error: Invalid index type "Tuple[slice, None]" for "Union[str, Tuple[int, int, int]]"; expected type "Union[int, slice]" [index] img_to_draw[:, mask] = color[:, None] ^ torchvision/utils.py:298: error: Invalid tuple index type (actual type "Tuple[slice, None]", expected type "Union[int, slice]") [misc] img_to_draw[:, mask] = color[:, None] ^ torchvision/utils.py:298: error: Incompatible types in assignment (expression has type "Union[str, Any]", target has type "Union[Tensor, Union[int, float, bool]]") [assignment] img_to_draw[:, mask] = color[:, None] ^ Found 4 errors in 1 file (checked 127 source files) Exited with code exit status 1

The easiest fix was to rename, rather than ignoring mypy errors.

Ok, let's leave it as-is then.

@pmeier what is your take on this?

Given that this is one of the most frustrating things when working with mypy, we could simply put allow_redefinition = True in our mypy config. I also proposed doing this in #4513, albeit only for the prototype datasets.

NicolasHug · 2021-10-04T07:31:46Z

torchvision/utils.py

-            be represented as PIL strings e.g. "red" or "#FF00FF", or as RGB tuples e.g. ``(240, 10, 157)``.
+        colors: Union[List[Union[str, Tuple[int, int, int]]], str, Tuple[int, int, int]]: List containing the colors
+            of the masks or single color for all masks. The colors can be represented as
+            PIL strings e.g. "red" or "#FF00FF", or as RGB tuples e.g. ``(240, 10, 157)``.
            When ``masks`` has a single entry of shape (H, W), you can pass a single color instead of a list


let's just remove this sentence below then, it doesn't add much to the description now

When masks has a single entry of shape (H, W), you can pass a single color instead of a list with one element

torchvision/utils.py

NicolasHug

Thanks @oke-aditya , LGTM with a last comment

torchvision/utils.py

Co-authored-by: Nicolas Hug <[email protected]>

NicolasHug

thanks, I'll merge when green

NicolasHug · 2021-10-04T14:31:15Z

@oke-aditya we just started formatting the code with black - you'll get a few merge conflicts as you already noticed. Don't worry about it, I'll take care of it.

we'll soon publish proper instructions on how to merge these conflicts, but until then feel free to ignore them :)

sorry for the unexpected conflicts!

torchvision/utils.py

Small docstring fix

NicolasHug · 2021-10-05T12:43:03Z

@oke-aditya if that's OK I'll merge #4531 first so we can remove the renaming from this PR, and then I'll merge it. Sounds good?

torchvision/utils.py

oke-aditya · 2021-10-05T12:45:28Z

Yeah works fine 😃

NicolasHug · 2021-10-05T14:59:15Z

Done! Thanks @oke-aditya

github-actions · 2021-10-05T15:21:51Z

Hey @NicolasHug!

You merged this PR, but no labels were added.

Summary: * Add str param * Update test to include str * Fix mypy * Remove a small bracket * Test more robustly * Update docstring and test: * Apply suggestions from code review * Update torchvision/utils.py Small docstring fix * Update torchvision/utils.py * remove unnecessary renaming Reviewed By: NicolasHug Differential Revision: D31505551 fbshipit-source-id: 459123a3774b39ee071f43ca2b196bd50319a60a Co-authored-by: Nicolas Hug <[email protected]> Co-authored-by: Nicolas Hug <[email protected]> Co-authored-by: Nicolas Hug <[email protected]>

* Add str param * Update test to include str * Fix mypy * Remove a small bracket * Test more robustly * Update docstring and test: * Apply suggestions from code review Co-authored-by: Nicolas Hug <[email protected]> * Update torchvision/utils.py Small docstring fix * Update torchvision/utils.py * remove unnecessary renaming Co-authored-by: Nicolas Hug <[email protected]> Co-authored-by: Nicolas Hug <[email protected]>

oke-aditya added 2 commits October 2, 2021 22:54

Add str param

6cb46e4

Update test to include str

663fa4c

pytorch-probot bot added the ciflow/default label Oct 2, 2021

facebook-github-bot added the cla signed label Oct 2, 2021

Fix mypy

2c34a52

oke-aditya added 2 commits October 3, 2021 01:14

Remove a small bracket

b9da8f3

Test more robustly

0482b7a

NicolasHug reviewed Oct 4, 2021

View reviewed changes

NicolasHug mentioned this pull request Oct 4, 2021

Set allow_redefinition = True for mypy #4531

Merged

oke-aditya added 2 commits October 4, 2021 17:42

Merge branch 'main' of https://github.com/pytorch/vision into fix_annot

be63a15

Update docstring and test:

dec7270

NicolasHug reviewed Oct 4, 2021

View reviewed changes

torchvision/utils.py Outdated Show resolved Hide resolved

torchvision/utils.py Outdated Show resolved Hide resolved

Apply suggestions from code review

de943a9

Co-authored-by: Nicolas Hug <[email protected]>

oke-aditya requested a review from NicolasHug October 4, 2021 12:18

NicolasHug approved these changes Oct 4, 2021

View reviewed changes

oke-aditya added 2 commits October 4, 2021 19:56

Merge branch 'main' of https://github.com/pytorch/vision into fix_annot

9f49b23

Merge branch 'fix_annot' of github.com:oke-aditya/vision into fix_annot

bda9132

oke-aditya commented Oct 5, 2021

View reviewed changes

torchvision/utils.py Outdated Show resolved Hide resolved

oke-aditya added 2 commits October 5, 2021 18:09

Update torchvision/utils.py

456bc92

Small docstring fix

Merge branch 'main' into fix_annot

7723858

NicolasHug reviewed Oct 5, 2021

View reviewed changes

torchvision/utils.py Outdated Show resolved Hide resolved

Update torchvision/utils.py

2489fea

NicolasHug added 3 commits October 5, 2021 14:47

Merge branch 'main' of github.com:pytorch/vision into fix_annot

10b4e7e

remove unnecessary renaming

36683cb

Merge branch 'fix_annot' of github.com:oke-aditya/vision into fix_annot

8295044

NicolasHug merged commit a75dc89 into pytorch:main Oct 5, 2021

oke-aditya deleted the fix_annot branch October 5, 2021 14:59

NicolasHug added code quality module: documentation module: utils labels Oct 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix annotation of draw_segmentation_masks #4527

Fix annotation of draw_segmentation_masks #4527

oke-aditya commented Oct 2, 2021 •

edited

Loading

oke-aditya commented Oct 2, 2021 •

edited

Loading

NicolasHug left a comment

NicolasHug Oct 4, 2021

oke-aditya Oct 4, 2021

NicolasHug Oct 4, 2021

oke-aditya Oct 4, 2021

NicolasHug Oct 4, 2021

pmeier Oct 4, 2021

NicolasHug Oct 4, 2021 •

edited

Loading

NicolasHug Oct 4, 2021

NicolasHug left a comment

NicolasHug left a comment

NicolasHug commented Oct 4, 2021

NicolasHug commented Oct 5, 2021

oke-aditya commented Oct 5, 2021

NicolasHug commented Oct 5, 2021

github-actions bot commented Oct 5, 2021

Fix annotation of draw_segmentation_masks #4527

Fix annotation of draw_segmentation_masks #4527

Conversation

oke-aditya commented Oct 2, 2021 • edited Loading

oke-aditya commented Oct 2, 2021 • edited Loading

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug Oct 4, 2021

Choose a reason for hiding this comment

oke-aditya Oct 4, 2021

Choose a reason for hiding this comment

NicolasHug Oct 4, 2021

Choose a reason for hiding this comment

oke-aditya Oct 4, 2021

Choose a reason for hiding this comment

NicolasHug Oct 4, 2021

Choose a reason for hiding this comment

pmeier Oct 4, 2021

Choose a reason for hiding this comment

NicolasHug Oct 4, 2021 • edited Loading

Choose a reason for hiding this comment

NicolasHug Oct 4, 2021

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug commented Oct 4, 2021

NicolasHug commented Oct 5, 2021

oke-aditya commented Oct 5, 2021

NicolasHug commented Oct 5, 2021

github-actions bot commented Oct 5, 2021

oke-aditya commented Oct 2, 2021 •

edited

Loading

oke-aditya commented Oct 2, 2021 •

edited

Loading

NicolasHug Oct 4, 2021 •

edited

Loading