Skip to content

support grayscale / RGB alpha conversions #5567

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

Merged
merged 17 commits into from
Mar 9, 2022
Merged

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Mar 8, 2022

Closes #5510.

@facebook-github-bot
Copy link

facebook-github-bot commented Mar 8, 2022

💊 CI failures summary and remediations

As of commit 803bc9a (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

Comment on lines 163 to 188
@parametrize(
[
(
transforms.ConvertImageColorSpace(color_space=new_color_space, old_color_space=old_color_space),
itertools.chain.from_iterable(
[
fn(color_spaces=[old_color_space])
for fn in (
make_images,
make_vanilla_tensor_images,
make_pil_images,
)
]
),
)
for old_color_space, new_color_space in itertools.product(
[
features.ColorSpace.GRAYSCALE,
features.ColorSpace.GRAYSCALE_ALPHA,
features.ColorSpace.RGB,
features.ColorSpace.RGBA,
],
repeat=2,
)
]
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: It is not only related to this one but for previous as well. IMO these parametrizations are really hard to read and understand... If we could simplify that somehow, it would be nice. Major difficulty is get what is passed to input for example...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is indeed hard to read. Refactoring the tests is at the top of my todo list.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

@pmeier Thanks! Overall looks good. Only couple of comments for your attention.

@pmeier pmeier requested a review from datumbox March 9, 2022 13:20
@pmeier pmeier requested a review from datumbox March 9, 2022 15:06
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@pmeier pmeier merged commit cddad9c into pytorch:main Mar 9, 2022
@pmeier pmeier deleted the alpha-conversion branch March 9, 2022 15:43
facebook-github-bot pushed a commit that referenced this pull request Mar 15, 2022
Summary:
* support grayscale / RGB alpha conversions

* use _max_valu from stable

* remove extra copy for PIL conversion

* simplify test image generation for color spaces with alpha channel

* use common _max_value in tests

* replace dynamically created dicts with if/else

* make color space conversion more explicit

* make even more explicit

* simplify alpha image generation

* fix if / elif

* add error for unknown conversions

* rename RGBA to RGB_ALPHA

* cleanup

* GRAYSCALE to GRAY

Reviewed By: vmoens

Differential Revision: D34878979

fbshipit-source-id: 4ec3af94e73152d11017c4e9c57ded44b2076764
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.

[RFC] How do we want to deal with images that include alpha channels?
4 participants