Skip to content

Make Image.__array__ take optional dtype argument#5572

Merged
radarhere merged 3 commits intopython-pillow:masterfrom
t-vi:__array__-dtype
Jul 5, 2021
Merged

Make Image.__array__ take optional dtype argument#5572
radarhere merged 3 commits intopython-pillow:masterfrom
t-vi:__array__-dtype

Conversation

@t-vi
Copy link
Copy Markdown
Contributor

@t-vi t-vi commented Jul 1, 2021

This is required by the numpy protocol.

Fixes #5571

@t-vi
Make Image.__array__ take optional dtype argument
1c03526 
This is required by the numpy protocol.
@radarhere radarhere added the NumPy label Jul 1, 2021
radarhere
radarhere reviewed Jul 1, 2021
View reviewed changes
@t-vi
Copy link
Copy Markdown
Contributor Author

t-vi commented Jul 1, 2021 •
edited by radarhere
Loading

To demystify what is needed by __array__, here are the two calls numpy does, one with dtype one without. (And in particular, you don't need to implement numpy.array 's signature.)

https://github.com/numpy/numpy/blob/45c57078fd599194b49148fb66808f03490ee423/numpy/core/src/multiarray/ctors.c#L2453-L2458

This has been very stable and the reason the bug shows up in Pillow 8.3 is that it implemented only the version without the dtype passed in when switching to providing __array__ in #5379.

I looked at numpy's ndarray __array__ for inspiration that providing a non-identical dtype casts/copies rather than failing.

radarhere
radarhere reviewed Jul 1, 2021
View reviewed changes
t-vi and others added 2 commits July 1, 2021 14:06
@t-vi
Use numpy.float64 instead of numpy.float to avoid deprecation (thank …
73f6cf2 
…you radarhere)
@t-vi @radarhere
Use numpy.array with dtype
8d1b433 
Thank you @radarhere.

Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
@t-vi
Copy link
Copy Markdown
Contributor Author

t-vi commented Jul 1, 2021 •
edited
Loading

I'm not entirely sure what to do with the codecov. It seems implausible to me that the line it points out in the Image.__array__ isn't covered by tests and I don't know what to think about the idea of tests being covered by tests.

@radarhere
Copy link
Copy Markdown
Member

radarhere commented Jul 1, 2021

I expect that was just because some CI jobs had an intermittent failure and were not returning their coverage results - everything looks fine with your latest commit.

@NicolasHug NicolasHug mentioned this pull request Jul 1, 2021
Closed
@t-vi
Copy link
Copy Markdown
Contributor Author

t-vi commented Jul 1, 2021

OK, I leave the it to you to take this forward, then. Thank you for your suggestions.

@Dref360 Dref360 mentioned this pull request Jul 2, 2021
Open
4 tasks
@ptrblck ptrblck mentioned this pull request Jul 2, 2021
Closed
@AdieResearch AdieResearch mentioned this pull request Jul 2, 2021
Closed
@mhash1m mhash1m mentioned this pull request Jul 2, 2021
Merged
@jni jni mentioned this pull request Jul 4, 2021
Closed
@radarhere radarhere merged commit d43d446 into python-pillow:master Jul 5, 2021
@hugovk hugovk mentioned this pull request Jul 5, 2021
Closed
23 tasks
@mikegerber mikegerber mentioned this pull request Jul 5, 2021
Closed
@dstein64 dstein64 mentioned this pull request Jul 5, 2021
Closed
jni added a commit to regro-cf-autotick-bot/scikit-image-feedstock that referenced this pull request Jul 7, 2021
@jni
Don't allow pillow 8.3.0
006702b 
See scikit-image/scikit-image#5455
and python-pillow/Pillow#5572
@hackgoofer hackgoofer mentioned this pull request Jul 8, 2021
Closed
@pacospace pacospace mentioned this pull request Jul 8, 2021
Closed
czlee added a commit to czlee/jadeite that referenced this pull request Jul 29, 2021
@czlee
Lock Pillow to version 8.2.0
a21471c 
There's currently a bug that interferes with the .to_tensor() function
in torch.

see: pytorch/pytorch#61125 (comment)
and: python-pillow/Pillow#5572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@radarhere radarhere radarhere left review comments

@hugovk hugovk hugovk approved these changes

Assignees

No one assigned

Labels

NumPy

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pillow 8.3 and NumPy

3 participants

@t-vi @radarhere @hugovk