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

Translate encoder error codes to strings; deprecate ImageFile.raise_oserror() #7609

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Dec 8, 2023

When decoding, we use raise_oserror() to convert codec error codes to strings. Adapt that code to be used when encoding as well. Add a new helper function that returns the exception so we can still raise from exc.

Replace all callers of raise_oserror() with raise _get_oserror(). Deprecate raise_oserror(), since it doesn't need to be in the public API.

Split out from #7553 (comment).

src/PIL/ImageFile.py Outdated Show resolved Hide resolved
@bgilbert
Copy link
Contributor Author

Should we switch all the internal callers to _get_oserror() and deprecate raise_oserror() entirely? It seems like the functionality is only useful for code that calls into libImaging, i.e. not for external callers. Its predecessor raise_ioerror() was inherited from PIL as an exported function.

Alternatively, if we don't want to do that, should we expose the new encoder argument on raise_oserror()? It's a bit awkward that raise_oserror() will have less functionality than the helper function it wraps.

@radarhere
Copy link
Member

Alternatively, if we don't want to do that, should we expose the new encoder argument on raise_oserror()? It's a bit awkward that raise_oserror() will have less functionality than the helper function it wraps.

That wouldn't completely simplify things though, would it? Because it is still ideal to raise the error from another in the following code

raise _get_oserror(errcode, encoder=True) from exc

which raise_oserror() can't do, with or without an encoder argument, because doesn't return an error, it immediately raises it.

While Pillow values backwards compatibility, I agree that raise_oserror() doesn't appear useful to users, so deprecating it sounds fine to me.

@bgilbert bgilbert changed the title Translate encoder error codes to strings Translate encoder error codes to strings; deprecate raise_oserror() Dec 10, 2023
@bgilbert
Copy link
Contributor Author

Sounds good, done.

docs/deprecations.rst Outdated Show resolved Hide resolved
src/PIL/_deprecate.py Outdated Show resolved Hide resolved
src/PIL/ImageFile.py Outdated Show resolved Hide resolved
When decoding, we use raise_oserror() to convert codec error codes to
strings.  Adapt that code to be used when encoding as well.  Add a new
helper function that returns the exception so we can still raise
`from exc`.
It's only useful if the caller has an IMAGING_CODEC_* error code, which
are only produced by codec decode() methods and are automatically
translated by ImageFile.

Co-authored-by: Andrew Murray <radarhere@users.noreply.github.com>
@bgilbert
Copy link
Contributor Author

Added @radarhere's changes from bgilbert#4.

@radarhere radarhere changed the title Translate encoder error codes to strings; deprecate raise_oserror() Translate encoder error codes to strings; deprecate ImageFile.raise_oserror() Dec 13, 2023
@radarhere radarhere merged commit 45e4408 into python-pillow:main Dec 13, 2023
57 checks passed
@bgilbert bgilbert deleted the encoder-errors branch December 13, 2023 09:46
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.

3 participants