clarifying docs for ToPILImage() #7679
Conversation
|
|
||
| This transform does not support torchscript. | ||
|
|
||
| Converts a torch.*Tensor of shape C x H x W or a numpy ndarray of shape | ||
| H x W x C to a PIL Image while preserving the value range. | ||
| H x W x C to a PIL Image while adjusting the value range depending on the ``mode``. |
There was a problem hiding this comment.
@NicolasHug opinion on whether we even want to mention that?
| H x W x C to a PIL Image while adjusting the value range depending on the ``mode``. | |
| H x W x C to a PIL Image. |
There was a problem hiding this comment.
I'm OK with it. It is slightly more accurate than what we previously had, even though arguably it doesn't provide a ton of info right away, since readers would need to know the details of each PIL mode to understand what's happening. Perhaps we can add that info next to each mode below, e.g.
- If the input has 3 channels, the ``mode`` is assumed to be ``RGB`` (uint8 values in [0, 255])
etc.
There was a problem hiding this comment.
Sure, will make the required changes.
There was a problem hiding this comment.
@NicolasHug @pmeier All the modes are not mentioned explicitly in the docs. PIL.Image mode is referred to that in the docs. So won't it be better if we just write this?
Converts a torch.*Tensor of shape C x H x W or a numpy ndarray of shape
H x W x C to a PIL Image while keeping the value range consistent with PIL.Image modes.
There was a problem hiding this comment.
It wouldn't address that part of my comment above:
even though arguably it doesn't provide a ton of info right away, since readers would need to know the details of each PIL mode to understand what's happening
But that's OK, we can provide more detail incrementally. Let's just merge this PR the way it is right now.
@sahilg06 do you mind reverting the changes made in https://github.com/pytorch/vision/pull/7679/files#r1234422939 so we can merge it? Thanks!
torchvision/transforms/transforms.py
Outdated
| @@ -199,21 +199,21 @@ def forward(self, image): | |||
|
|
|||
|
|
|||
| class ToPILImage: | |||
| """Convert a tensor or an ndarray to PIL Image - this does not scale values. | |||
| """Converts a tensor or an ndarray to PIL Image | |||
There was a problem hiding this comment.
Seems like python docstring convention suggests to use imperative verb instead of conjugated verb
Source: https://peps.python.org/pep-0257/
It prescribes the function or method’s effect as a command (“Do this”, “Return that”), not as a description; e.g. don’t write “Returns the pathname …”.
There was a problem hiding this comment.
Okay, but at here also, it's written "Applies." There may be more such cases.
There was a problem hiding this comment.
These are conventions that may be not respected everywhere for a certain reason. In torchvision (cc @NicolasHug ) we want to follow these convention and in the examples where it is not the case we may want to fix that. Anyway let's not introduce another convention here.
Summary: Co-authored-by: Nicolas Hug <nh.nicolas.hug@gmail.com> Reviewed By: vmoens Differential Revision: D47186568 fbshipit-source-id: e18b4e33bd621d4c6b38bba3f6c6c405bad35cc5
cc @vfdev-5