Repository: pytorch/vision
Pull request: add warning if font is not set
Fixes #5725


Problem statement:
torchvision.utils.draw_bounding_boxes set font_size
### 🚀 The feature

font_ The size setting must be specified at the same time as the font to be valid, otherwise it is invalid.
I think utils.py in torchvision Line 239 can be improved.Make font settings and font size settings separate.
utils.py Line 239:
"txt_font = ImageFont.load_default() if **font is None** else ImageFont.truetype(font=font, size=font_size)"

### Motivation, pitch

Improve the experience of torchvision.draw_bounding_boxes

### Alternatives

_No response_

### Additional context

_No response_

Hints from issue discussion:
If I understand correctly, You want `font_size` to work for default font right? 
But I believe the PIL Does not have this functionality to load default font with a given font size

See https://github.com/python-pillow/Pillow/issues/2695



Thank you. I see.
Given that this is just a small utility that we provide and `PIL` does not have this option, I don't think there is much that we can do for now. Still, thanks for the request @haozhihang123 and @oke-aditya for linking the relevant issue!
Although one way is to Have a default font from torchvision side and not rely on OS. Which would allow to set `font_size`

I won't recommend this fix. As the current API works. 

It might be wiser to throw a warning if user supplies `font_size` without specifying font. 
> It might be wiser to throw a warning if user supplies `font_size` without specifying font.

Yes, that is indeed a good change. Whenever user input gets ignore for any reason, we should warn about it. @oke-aditya do you want to send a PR?
I believe some new contributor could take this. As it's a good first issue :) 

(Of course I can submit a PR. But will feel more happy to see a new contributor)
hey @pmeier I can take this up.
feel free to have a go

Commit message: Apply suggestions from code review

Co-authored-by: Philip Meier <github.pmeier@posteo.de>

Patch to review:
diff --git a/test/test_utils.py b/test/test_utils.py
index 90d4fdf8552..18b15f80508 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -168,6 +168,13 @@ def test_draw_invalid_boxes():
         utils.draw_bounding_boxes(img_correct, boxes, colors=colors_wrong)
 
 
+def test_draw_boxes_warning():
+    img = torch.full((3, 100, 100), 255, dtype=torch.uint8)
+
+    with pytest.warns(UserWarning, match="Argument 'font_size' will be unused as 'font' is not set."):
+        utils.draw_bounding_boxes(img, boxes, fill=True, font_size=11)
+
+
 @pytest.mark.parametrize(
     "colors",
     [
diff --git a/torchvision/utils.py b/torchvision/utils.py
index 4737a047327..78574ea2b4c 100644
--- a/torchvision/utils.py
+++ b/torchvision/utils.py
@@ -164,7 +164,7 @@ def draw_bounding_boxes(
     fill: Optional[bool] = False,
     width: int = 1,
     font: Optional[str] = None,
-    font_size: int = 10,
+    font_size: Optional[int] = None,
 ) -> torch.Tensor:
 
     """
@@ -223,6 +223,12 @@ def draw_bounding_boxes(
 
     colors = [(ImageColor.getrgb(color) if isinstance(color, str) else color) for color in colors]
 
+    if font is None and font_size is not None:
+        warnings.warn("Argument 'font_size' will be ignored since 'font' is not set.")
+        font_size = None
+    elif font is not None and font_size is None:
+        font_size = 10
+
     # Handle Grayscale images
     if image.size(0) == 1:
         image = torch.tile(image, (3, 1, 1))


Code review comments:
diff_hunk: @@ -168,6 +168,13 @@ def test_draw_invalid_boxes():
         utils.draw_bounding_boxes(img_correct, boxes, colors=colors_wrong)
 
 
+def test_draw_boxes_warning():
+    img = torch.full((3, 100, 100), 255, dtype=torch.uint8)
+
+    with pytest.warns(UserWarning, match="Argument 'font_size' will be unused as 'font' is not set."):
path: test/test_utils.py
text: @user1:
The string passed to `match` is actually a regex pattern. Thus, if you have special regex characters like `.` in it, you should wrap it in `re.escape` to mark them as "regular". For example

```py
>>> import re
>>> match = "'font' is not set."
>>> re.escape(match)
"'font'\\ is\\ not\\ set\\."
```

```suggestion
    with pytest.warns(UserWarning, match=re.escape("Argument 'font_size' will be ignored since 'font' is not set.")):
```

Note that you will have to `import re` at the top of the module.

@author:
Noted. Will add the change. 👍 

@user1:
@author You forgot to update the warning message to match the one that is emitted from the function.

@author:
Sorry. Missed noticing this somehow. Have updated it now.