Skip to content

Conversation

@amyeroberts
Copy link
Contributor

What does this PR do?

Adds normalize to the image transforms modules, as well as a helper utility function get_channel_dimension_axis.

  • normalize: performs equivalent normalization of an image as previous feature extractors
  • get_channel_dimension_axis: Helper function which returns which axis number the channel dimension is on.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 12, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, it's very clean!

The channel dimension format of the output image. If `None`, will use the inferred format from the input.
"""
if isinstance(image, PIL.Image.Image):
warnings.warn("PIL will not be supported as input in the next release. Please use numpy arrays instead.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
warnings.warn("PIL will not be supported as input in the next release. Please use numpy arrays instead.")
warnings.warn("PIL will not be supported as input in the next release. Please use numpy arrays instead.", FutureWarning)

Also, please be explicit about the version: is it 4.25.0 (since we're developing 4.24.0 right now) or 5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Do we have a rule of thumb for how many releases we normally support?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Normally we only do breaking changes at major releases, but for this one we're making an exception with a short deprecation cycle. Just checked with Lysandre and we should support the deprecation for two minors and fully remove this in 4.26.0 if that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, makes sense! I'll update the warning message.

Copy link
Contributor

@alaradirik alaradirik left a comment

Choose a reason for hiding this comment

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

Everything looks good to me!

Copy link
Contributor

@NielsRogge NielsRogge left a comment

Choose a reason for hiding this comment

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

Thanks for adding! Looks good to me.

@amyeroberts amyeroberts merged commit 4181320 into huggingface:main Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.