sylikc / pyexiftool

PyExifTool (active PyPI project) - A Python library to communicate with an instance of Phil Harvey's ExifTool command-line application. Runs one process with special -stay_open flag, and pipes data to/from. Much more efficient than running a subprocess for each command!
Other
144 stars 17 forks source link

Writing lists of values #12

Closed davidorme closed 3 years ago

davidorme commented 3 years ago

I'm trying to use pyexiftool to update Keywords and Subject, which are both 'list' tags. From the exiftool docs, it looks like the choice is to use one of these approaches:

 exiftool -Keywords=keyword1 -Keywords=keyword2 -Keywords=keyword3 file.jpg
 exiftool -sep ", " -Keywords="keyword1, keyword2, keyword3" file.jpg

I can't see a way to get this to work from pyexiftool. For option 1, the set_tags_batch function takes a dictionary of tags and values, and so obviously can't contain multiple instances of the same tag as keys. For option 2, I've tried setting:

extl = exiftool.ExifTool(common_args=['-sep', '", "'])
extl.start()
extl.set_tags({'Keywords': "keyword1, keyword2, keyword3" }, 'file.jpg')

That doesn't work either - when you read the file it is a single string and not a list of three elements. I've tried using a list as the values:

extl.set_tags({'Keywords': ["keyword1", "keyword2", "keyword3"]}, 'file.jpg')

But that just ends up using the repr of the list. Any suggestions?

davidorme commented 3 years ago

Just to follow up - I can see that the set_keywords function works with a list, but is specific to the 'IPTC:Keywords' tag. I could always overwrite the global variable KW_TAGNAME to target another list field, but only a single variable can be handled at a time. I've tried modifying set_tags_batch as follows:

                params = []
                params_utf8 = []
                for tag, value in tags.items():
                        if isinstance(value, list):
                                for item in value:
                                        params.append(u'-%s=%s' % (tag, item))
                        else:
                                params.append(u'-%s=%s' % (tag, value))

That works as expected. I've also tried passing a list to a non-list tag (e.g. XMP:Coverage) after that modification and exiftool handles that sanely - the EXIF tag only retains the last value in the list.

sylikc commented 3 years ago

The set_keywords function was merged from some pull request back in the old repo. I guess it wasn't considered to have multiple repeated tags with the same name, but different values.

Your fix above is probably the best way to fix it right now. I think ultimately, having the tags be lists implies that the tag should be duplicated is probably the cleanest way to go.

If you would like to create PR, so that your contributing info gets forever embedded into the git repo history, you can... or I can just use the fix above and refer to this issue. Let me know which way you'd prefer.

(FYI: I'm also actively developing on a separate branch which is going to be Python 3.6+ and I'll see if there's a good way to do this generically.) The docs are also a bit outdated given the changes I merged in...

davidorme commented 3 years ago

That was my thinking too - list (and maybe tuples?) imply multiple values. It would be nice if there was a warning when writing a list to a non-list field, but exiftool doesn't register that as a minor error, and trapping it would need to access the description of all fields. That seems out of scope!

I'm happy for the code to be adopted and the issue referenced!

sylikc commented 3 years ago

ok, I just merged the change as described. do you need a pip release?

davidorme commented 3 years ago

Not urgently. I've patched my local copy and at the moment I'm the only one in my group that is likely to be needing the patch. Thanks!

sylikc commented 3 years ago

released the version with this patch on PyPI https://pypi.org/project/PyExifTool/0.4.7/

thanks for the contribution!