Skip to content

Conversation

@NicolasHug
Copy link
Member

This PR adds docs for the v2 transforms that already exist in v1. I haven't tackled the new ones yet.

I jsut copy/pasted the docstrings and added:

  • the [BETA] header in the first line
  • the .. betastatus:: sphinx directive
  • replace "image" with "image / box / mask" where relevant.

v2.ToPILImage
v2.ToImagePIL
ToTensor
v2.ToTensor
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it is deprecated, should we document it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that ToImageTensor is missing from the list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since it is deprecated, should we document it?

Some thoughts here https://github.com/pytorch/vision/pull/7297/files#r1113244517

It seems that ToImageTensor is missing from the list.

I did not add any new v2 transforms here, so that's normal

PILToTensor
v2.PILToTensor
ConvertImageDtype
v2.ConvertImageDtype
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. Just an alias for ConvertDtype



class Grayscale(Transform):
"""[BETA] Convert image to grayscale.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""[BETA] Convert image to grayscale.
"""[BETA] Convert images or videos to grayscale.

Copy link
Contributor

Choose a reason for hiding this comment

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

Applies to every color transform

Copy link
Member Author

Choose a reason for hiding this comment

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

(un-resolving because I didn't apply that to the rest of the color transforms yet)



class RandomHorizontalFlip(_RandomApplyTransform):
"""[BETA] Horizontally flip the given image/box/mask randomly with a given probability.
Copy link
Contributor

Choose a reason for hiding this comment

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

video is missing here as well

Comment on lines +724 to +726
.. note::
In torchscript mode padding as single int is not supported, use a sequence of
length 1: ``[padding, ]``.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comes up multiple times. Same question as above: should we document this at all?

Copy link
Contributor

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

stamped

@NicolasHug
Copy link
Member Author

Thanks @pmeier @vfdev-5 for the review - I'll merge now to unblock upcoming PRs, but there are outstanding comments that I haven't addressed (yet) in this PR. We'll address them in the next few PRs to come. I marked the alread-addressed comments as resolved, so we just need to check the un-resolved ones.

@NicolasHug NicolasHug merged commit 928b05c into pytorch:main Feb 21, 2023
@github-actions
Copy link

Hey @NicolasHug!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

NicolasHug added a commit to NicolasHug/vision that referenced this pull request Feb 24, 2023
facebook-github-bot pushed a commit that referenced this pull request Mar 29, 2023
Summary:

Reviewed By: vmoens

Differential Revision: D44416569

fbshipit-source-id: 03689b20c6de2afe3971b3e41e1f74ef1845b53f

Co-authored-by: vfdev <[email protected]>
Co-authored-by: Philip Meier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants