-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add inverse mappings for exif tags #6586
Conversation
A nicer solution would be to switch to an enum with the right values, but this is the quick win.
for more information, see https://pre-commit.ci
:type: dict | ||
|
||
The GPS_CODES dictionary maps descriptive string names to 8-bit integer EXIF | ||
gps enumerations. For instance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gps enumerations. For instance: | |
GPS enumerations. For instance: |
Could you describe an example use case for this feature? The current dictionary allows for the codes to be translated into something that can be understood by humans. What is your particular scenario where you want to go the other way? |
Could you expand on this? |
Suppose a user wants to set an exif tag, I presume they would want to set the data using a human readable tag name. |
Using a dict to store these values feels hacky in the first place. from enum import IntEnum
class ExifTags(IntEnum):
InteropIndex = 0x0001
ProcessingSoftware = 0x000B
[...] Which gives us just the right access: $ExifTags(0x0001).name
InteropIndex
$ExifTags.InteropIndex.value
1 |
To elaborate a bit more. Currently exif_data = Image.Exif()
exif_data[0x123456] = '2020:01:01 00:00:00'
image.save(filename, exif=exif_data.tobytes()) People have to manage how to keep it readable themselves. Their options include adding a comment, rolling their own inverse mapping or accepting less readable code. After this pull request exif_data = Image.Exif()
exif_data[TAG_CODES['DateTimeOriginal']] = '2020:01:01 00:00:00'
image.save(filename, exif=exif_data.tobytes()) Here we get the mapping for free, so we don't need to worry about the numerical exif ID (which, to the user, is an implementation detail). The code will crash if there is a typo in the name (which is a good thing) but your favorite IDE may have trouble autocompleting a tag name. After bigger (breaking?) enum change exif_data = Image.Exif()
exif_data[ExifTags.DateTimeOriginal] = '2020:01:01 00:00:00'
image.save(filename, exif=exif_data.tobytes()) Here your IDE will autocomplete the tagname. May need to update the code of ExifTags a bit to do this right. Could provide a TAGS dict generated from the Enum for backward compatibility. |
I'm in favour of this, but when we previously had this conversation in #5875, I implemented it, it was suggested that the original attributes be deprecated, and now there's #6537 where a user doesn't like that we changed the API. So I might hold off on this for a moment. |
Whenever adding anything to an API, it is important to think about a potential future cost of deprecating it again. Deprecation of the old approach would be costly indeed. It is always a balance between quality of a library and the amount of maintenance for existing users. It is never possible to please everyone, but we can try our best. For a change like the one suggested in this thread, backward compatibility should win. Adding the Enum in a backward compatible way is possible however. I'm sorry about not noticing the backward compatibility break in #5954. In any case, thank you for your hard work on PIL! |
I've created PR #6630 to use enums as a possible alternative to this. |
#6630 has been merged. |
A nicer solution would be to switch to an enum with the right values, but this is the quick win.
Tested locally, ran through pycodestyle. To me it seems the code is trivial enough to not warrant adding a unit test.