-
Notifications
You must be signed in to change notification settings - Fork 21
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
execute_json should set the ExifTool param -struct too #52
Comments
@nitmws interesting observation, and thanks for the details report. Can you attach a file with these EXIF tags so I can analyze further whether using exiftool (utility) with different parameters might return different results (i'm thinking of testing grouping and stuff)? Do you think adding the @-struct@ parameter will have any potential impact on undesired behavior? Thanks |
@sylikc find attached some files:
Comparing the JSON files will show the big difference: the JSON of the *out_NOstruct file has only properties This difference will have an impact on processing the metadata because with the JSON of the *out_NOstruct file people have to collect a set of properties to generate the metadata of a Location Shown and if there are multiple they have to assign the first value of an array to the first Location Shown and the second value of the array to the second Location Show. Bad experience: the XMP:LocationShownCity property has only one value, to which of the two Locations should it be assigned ??? Showing the metadata of the JSON file with the -struct parameter this is much easier: the properties of the first Location Shown are the properties of the first object in the array of XMP's LocationShown and the second object in the array holds the properties of the second location, it is crystal clear which of the two properties are missing the name of a city. |
it's taken me awhile to get back to this... but after thinking it over, while this could potentially be a script-breaking change for downstream users, it's just the better thing to do. Much like some of the ExifToolAlpha() changes I had made making it clearer what info comes from what file, this is a good change even if it does break downstream users (until they adapt their code) |
After reading through the documentation it's still debatable whether to add the flag or not... the current design of PyExifTool makes minimal changes to the default Exiftool output. There's already tons of grief from people trying to figure out the '-G' and '-n' flags which are specified on default... (only one issue opened in this repo, but the upstream and stackoverflow have a good number) if specifying
in my own use case, I already pass if I replaced the default parameters to
I'm not sure what the right way to do this is... as a user of PyExifTool myself, I just specify |
I understand your concerns, @sylikc , regarding backward compatibility. We at IPTC are aware than many users of photo metadata prefer using simple properties without a structure. But the structured properties get more important, e.g. for telling from which URL a licensed image can be bought Google uses the Web URL sub-property of the structured Licensor property. Therefore I suggest this help by PyExiftool to implement the safe use of structured properties in an easy way:
|
I'm still thinking about your suggestion above... I am debating whether to add this helper function to the It would be relatively trivial to write an def execute_struct(self, *params):
return self.execute_json("-struct", *params) Have you also considered suggesting to the upstream |
To which class this function is added is up to you. Can a parameter set by such a function be overridden by the *params? The Regarding combining |
The Setting # does not work
with exiftool.ExifTool() as et:
print(et.execute_json('-G1', filepath))
# works
with exiftool.ExifTool(common_args=['-n']) as et:
print(et.execute_json('-G1', filepath))
# works
with exiftool.ExifTool(common_args=['-G1', '-n']) as et:
print(et.execute_json(filepath))
# works
et = exiftool.ExifToolHelper()
et.common_args = ['-G1', '-n']
print(et.execute_json(filepath))
I see... yeah, backwards compatibility ties the hands to how much can change with projects which have so many dependencies... I took a leap of faith last year when I totally chopped up PyExifTool #13 . Luckily, aside from code refactors, the intended output didn't change, and it looks like adoption is good. |
Currently calling execute_json sets only
-j
as ExifTool parameter - but it does not set the-struct
parameter. That's a dangerous for metadata properties with structured values and multiple values.Example: the IPTC Photo Metadata Standard defines a property Location Shown in the Image which has a structure of City, State/Province, Country Name, Country Code, Sublocation and more. And it may have multiple values = multiple structures.
Using the
-j
(JSON) parameter without the-struct
parameter returns such a result:Using the
-j
(JSON) parameter WITH the-struct
parameter returns such a result:The essential difference: the
XMP:LocationShownCity
and theXMP:LocationShownSublocation
of the result without-struct
have only a single value in the array, but knowbody knows if this is the City name or Sublocation name of the first location of of the second location. While the XMP:LocationShown has a JSON object for each location and the first object has no City but a Sublocation, the second object has a City but no Sublocation. Which location has what structured data is crystal clear.(Note: the results above are taken from the IPTC Photo Metadata reference image, it has values telling to which property it belongs.)
With this semantic issue as background I suggest to set the
-struct
parameter with the-j
parameter in the execute_json method.The text was updated successfully, but these errors were encountered: