Skip to content

Deprecate imread() reading from URLs #18649

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Deprecate imread() reading from URLs #18649

wants to merge 1 commit into from

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Oct 3, 2020

PR Summary

Closes #18648.

@timhoffm timhoffm added this to the v3.4.0 milestone Oct 3, 2020
@timhoffm timhoffm force-pushed the deprecate-imread-url branch 2 times, most recently from 96ac787 to 53a1b1f Compare October 4, 2020 22:00
@jklymak
Copy link
Member

jklymak commented Oct 5, 2020

recyclying to get doc build to rerun...

@jklymak jklymak closed this Oct 5, 2020
@jklymak jklymak reopened this Oct 5, 2020
@timhoffm timhoffm force-pushed the deprecate-imread-url branch from 53a1b1f to 5b132aa Compare October 5, 2020 21:28
QuLogic
QuLogic reviewed Oct 6, 2020
View reviewed changes
doc/api/next_api_changes/deprecations/18649-TH.rst
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Passing a URL to `~.pyplot.imread()` is deprecated. Please open the URL before
reading using ``urllib.request.urlopen()``.
Copy link
Member

Choose a reason for hiding this comment

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

As part of the standard library, shouldn't this be link-able?

@tacaswell
Copy link
Member

The user also has to do a bit more jiggery-pokery than just urlopen as pillow expects to be able to seek on the buffer and what comes back from urlopen does not support seek (see discussion in #18129).

The documented incantation needs to be image.imread(io.BytesIO(urllib.request.urlopen(url).read())) or image.imread(io.BytesIO(requests.get(url).content)).

I suspect we should also abandon most of the png guessing code and just let pillow infer as much as it wants to. I think this would make our code simpler (as it is almost a completed drop-through to pillow) and make it more functional (it will open things other than png from a url).

I am 👍 on deprecating this, but we need to provide more guidance about what to replace it with.

tacaswell
tacaswell requested changes Oct 6, 2020
View reviewed changes
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

Needs better docs on path away from imread.

Should probably also have, "Use PIL.image.open(urlopen(url))" as an option too.

@timhoffm
Copy link
Member Author

timhoffm commented Oct 6, 2020

👍 on the suggestions.

Anybody, feel free to amend. I'm not in word-picking mood right now.

@anntzer anntzer mentioned this pull request Jan 26, 2021
Merged
7 tasks
@jklymak
Copy link
Member

jklymak commented Jan 27, 2021

close in favour of #19367

@jklymak jklymak closed this Jan 27, 2021
@timhoffm timhoffm deleted the deprecate-imread-url branch January 27, 2021 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic left review comments

@tacaswell tacaswell tacaswell requested changes

@dopplershift dopplershift dopplershift approved these changes

@jklymak jklymak jklymak approved these changes

Assignees
No one assigned
Labels
Maintenance status: duplicate
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

Drop support for directly imread()ing urls.
5 participants
@timhoffm @jklymak @tacaswell @dopplershift @QuLogic