Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make get_image_size and get_image_num_channels public #4321

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Aug 26, 2021

The F._get_image_size() and F._get_image_num_channels() are quite useful methods to determine the size and channels of images and they are used across many classes. This PR makes them public.

cc @vfdev-5 @datumbox

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

Only thing I would consider if we are making it public is about the order of the elements being returned. I've always found it confusing with PIL to have the sizes be (w, h) instead of (h, w), and _get_image_size returns (w, h) to minimize the number of lines changed when we introduced Tensor support for the transforms.

I'm not sure if it's worth changing the order of returned elements, but it's something worth keeping in mind.

Thoughts?

@datumbox
Copy link
Contributor Author

For transparency, I had an offline chat with @fmassa to see if it's worth adding a new method that returns all 3 arguments as (c, h, w) and deprecate the old. Unfortunately there are a few places we use img_size=(w,h) in public APIs:

img_size = F._get_image_size(img)
ret = self.get_params(self.degrees, self.translate, self.scale, self.shear, img_size)

Thus we will merge this version.

@datumbox datumbox merged commit 96f6e0a into pytorch:main Aug 26, 2021
@datumbox datumbox deleted the transforms/get_image_size_public branch August 26, 2021 12:58
@praveern
Copy link

praveern commented Aug 27, 2021

Cannot seem to be able to access these functions anymore. Error: #4328

facebook-github-bot pushed a commit that referenced this pull request Sep 9, 2021
Reviewed By: fmassa

Differential Revision: D30793335

fbshipit-source-id: 90ec6a0da25cdcf62a359bddfdffa0e5678aa233
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