mattiasw
Handling of 0..unbounded cardinality fields such as iptc Keywords
Description
In issue #105 and pr #106, Keywords type was changed from NumberArrayTag to NumberArrayTag[]. However, I note that it is possible to get a single value or an array value back in the API, so this change really aligned with the most likely use of the field rather than being strictly correct.
I think there are two approaches to address this:
a) Normalise fields that can be singular or array values into array values
b) Change the types to correctly represent what the possible values may be, e.g. Type | Type[]
In either case it would be a breaking change, however, my preference would be to normalise the fields to always be array values as this is the most common case and the simplest to code against (I do this in my code base as a defensive measure against the variation).
Looking at https://iptc.org/std/photometadata/specification/IPTC-PhotoMetadata#keywords, I see that there quite a few other fields with cardinality of 0..unbounded, which makes me wonder if this is a more widespread concern in terms of the suggested approach. I would feel that 0..1 should be modelled as a singular optional field and 0..unbounded should be modelled an array with 0 or more entries.
Has this been discussed previously?
After a discussion and an agreement on approach, I'd be happy to work on it and submit a merge request.
Additional details
- ExifReader version: 4.5.1 (and many previous versions since #106 merged)
- Can you reproduce the bug on the examples site? If so, using which implementation (global object, AMD, and/or ES module)? https://mattiasw.github.io/ExifReader/
How to reproduce
- Try an image with a single Keyword
- Try an image with multiple Keywords
What happened:
In the first case a single object is returned and in the second case an array of objects is returned
What I expected would happen:
Either normalise to always return an array of objects, or change the types to show it could be an array or single value, e.g. NumberArrayTag[] | NumberArrayTag
Hi! Just want to say I've read this and will have a look as soon as I can, a lot of stuff happening right now. There was probably a reason why it's a single value if there is only one tag, I just have to remember what it was. 😅 I'll get back to you.
I agree, normalizing is probably the best route here. I vaguely recall some reasoning but can't really formulate it now. Plus, having it always return an array will make it easier for people to create a correct implementation in case they happen to only test with images with a single value.
If you'd like to make the change it's going to be in src/iptc-tags.js. Search for repeatable. That's how to know if the tag can have multiple values. The if statement has to be changed, and the else block. I think that may be it. The tests probably don't have to be updated which also is a sign that the single-value case is not important. After that the typings need to be changed for all repeatable IPTC tags. You can see which ones those are by looking for repeatable in the file src/iptc-tag-names.js.