Merged
Conversation
hugovk
reviewed
Dec 31, 2023
Member
|
This looks fine, please can you split this into two PRs, one to deprecate and the other to add type hints? I think it will be clearer to keep them separate. If you want, it's okay to leave this PR as-is, and create another one that is the first step, and we can merge that one first. |
nulano
changed the title
Contributor
Author
|
OK, I've removed the deprecation commits and created #7664 for that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Helps #2625.
There are two tricky parts in adding type hints to
PIL._binary:The
i16*andi32*give a "Returning Any from function declared to return int" in--strictmode due to the use ofstruct.unpack_from. This can be properly silenced with#type: ignore[no-any-return], but that gives a "Unused type ignore comment" when running without--strict, so I have left it out for now.The
i8function distinguishes betweenintandbytesinputs in a way that mypy doesn't recognize. Sincei8is called with anintinput only fromIptcImagePlugin, I have removed those calls there and removedintsupport as input toi8.Although the values in
self.info[(3, 60)]andself.info[(3, 65)]can be lists if the tags are duplicated, these tags are marked as "non-repeatable" in the specification (pg. 13-14). The specification recommends using the first entry if the tags are duplicated (sec. 1.5-d), but this recommendation was already ignored inIptcImagePluginand I was unable to find any images to test this with.In doing so, I found that
IptcImagePlugin.dump,IptcImagePlugin.i, andIptcImagePlugin.PADare simple helpers that should probably be deprecated (edit: #7664).