Skip to content
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

Remove deprecated PIL.OleFileIO in favour of olefile Python package #2784

Closed
wants to merge 1 commit into from

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Oct 4, 2017

The vendored version was removed in #2199, on 13 Dec 2016, in favour of the olefile Python package and replaced with a deprecation warning that PIL.OleFileIO would be removed in a future version.

We've had four quarterly releases since then (4.0.0, 4.1.0, 4.2.0, 4.3.0), shall we now finish this off?

cc: @decalage2 @jdufresne

@hugovk hugovk added the olefile label Oct 4, 2017
@wiredfool
Copy link
Member

Is this going to improve anyone's experience in using Pillow?

@hugovk
Copy link
Member Author

hugovk commented Nov 5, 2017

If they're still importing PIL/OleFileIO.py, then rather than the deprecation warning they'll get an import error and will finally, after a year of warnings, have to update their code to import from the real module.

@wiredfool
Copy link
Member

Well, yes, that's what the PR does. But does it improve their experience? It takes formerly working code and intentionally breaks it. And unlike some of our other deprecations, it removes all evidence that this was here without pointing to the problem that they are now having.

@jdufresne
Copy link
Contributor

And unlike some of our other deprecations, it removes all evidence that this was here without pointing to the problem that they are now having.

Would a mention in CHANGES.rst handle this concern?

@hugovk
Copy link
Member Author

hugovk commented Nov 5, 2017

We can keep this file longer (forever?), or replace it with an import error that says the same thing and then remove it later.

@jdufresne
Copy link
Contributor

or replace it with an import error that says the same thing and then remove it later.

I think this is a good compromise. It would still break code requiring someone to take action but would do so with a more helpful message.

@hugovk
Copy link
Member Author

hugovk commented Nov 6, 2017

#2833 has been merged, let's revisit this in the future.

@hugovk hugovk added this to the Future milestone Nov 6, 2017
@wiredfool wiredfool closed this Nov 6, 2017
@hugovk hugovk mentioned this pull request Jan 13, 2019
5 tasks
@hugovk hugovk deleted the rm-olefile-deprecation branch January 19, 2019 15:52
@hugovk hugovk added the Removal Removal of a feature, usually done in major releases label Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Documentation Needs Release Notes Removal Removal of a feature, usually done in major releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants