-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
transformers.image_transforms.normalize wrong types #35773
transformers.image_transforms.normalize wrong types #35773
Conversation
…wrong type for std and mean arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is correct. Thanks for the fix!
transformers.image_transforms.normalize documents and checks for the wrong type for std and mean arguments Co-authored-by: Louis Groux <louis.cal.groux@gmail.com>
transformers.image_transforms.normalize documents and checks for the wrong type for std and mean arguments Co-authored-by: Louis Groux <louis.cal.groux@gmail.com>
This PR was probably supposed to be a refactor, and should not change the execution paths of the code. However, it turns out that numpy arrays are Iterables but not Sequences, and therefore the execution paths changed. Client code that used to pass numpy arrays for from typing import Iterable, Sequence
import numpy as np
x = np.ones(3)
print(isinstance(x, Iterable))
#> True
print(isinstance(x, Sequence))
#> False I can see that the type hint does not include numpy arrays, but passing a numpy array (which I guess is the intention of the type hint Best :) Edit: I found that some client libs like vLLM are adjusting accordingly vllm-project/vllm@e8616d7 |
Oh, you're right, I went one level too high in the typing, the type needed was I will have a PR out tonight unless someone beats me to it.
|
Alright I created a fix and modified some tests to make sure it worked here |
What does this PR do?
transformers.image_transforms.normalize
typesstd
andmean
as Iterable, and checks for that, but callslen
on them. The proper type for a sized iterable isSequence
, this PR adds that.Fixes #35772
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.