Skip to content
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

Update make_string util to clean up bad values #128

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

EtiennePelletier
Copy link
Contributor

From exif-samples-master, the type of the seq arguments for a few entries (mostly UserComment) in util.make_string is a list. mypy still succeeds after the type annotation amendment. There is also no value in keeping a list of null bytes.

On another note, many other UserComment only contain spaces and some other fields contained trailing spaces (seen null character as well).

Finally, Nikon MakerNote ISOSetting is int16u[2] and should not be stringified. https://exiftool.org/TagNames/Nikon.html Sample values seen include [0, 0] and [0, 200], which were respectively being converted to '[0, 0]' and '}' by make_string.

The test diff output does not clearly show the effect of .strip(), so here it is along one example of the printable value of the IfdTag:

-  "EXIF UserComment": "                                                                                                                     ",
+  "EXIF UserComment": "",
$ diff -y --suppress-common-lines exif-samples-master/dump exif-samples-master/dump_test
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
MakerNote MakernoteVersion (Undefined): [0, 2, 0, 0]          | MakerNote MakernoteVersion (Undefined): 0200
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined): [0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  | EXIF UserComment (Undefined): 
EXIF UserComment (Undefined):                                 | EXIF UserComment (Undefined): 

@ianare, if you approve this PR, I can modify the exif-sample-master/dump afterwards to reflect changes :)

🐍 🤹‍♂️

@ianare
Copy link
Owner

ianare commented Nov 4, 2020

Great, thanks! If you can do the PR for the samples, that would be perfect. I'll approve this but wait for the samples to be done before merging to avoid false failures.

@ianare ianare self-requested a review November 4, 2020 20:41
@EtiennePelletier
Copy link
Contributor Author

Here we go: ianare/exif-samples#5

@ianare ianare merged commit 31d1d51 into ianare:develop Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants