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

loss of precision caused by json #76

Closed
Yang-z opened this issue Sep 4, 2023 · 6 comments · Fixed by #78
Closed

loss of precision caused by json #76

Yang-z opened this issue Sep 4, 2023 · 6 comments · Fixed by #78
Labels
bug Something isn't working

Comments

@Yang-z
Copy link

Yang-z commented Sep 4, 2023

Hi,

After I write a value e.g. 1.10 to the tag Exif:UserComment and read it by pyExifTool using method involving json (exiftoolhelper.get_tags()), the return value becomes 1.1, the precision just lost. (1.10 means 10th January for me, not equal to 1.1.)

According to exiftool doc:
exiftool OPTIONS

ExifTool quotes JSON values only if they don't look like numbers (regardless of the original storage format or the relevant metadata specification).

Additionally, the original outputs of exiftool do keep zeros at the end:

[{
  "SourceFile": "./test.jpg",
  "UserComment": 1.10
}]

So, I guess parse float as str is nessary. And the fellowing code could fix the problem:

//exiftool.py line 1159
parsed = json.loads(result, parse_float=str)

I think It's a good idea to add a option to let users to parse float as str without modifing the lib of pyexiftool.

@sylikc sylikc added the bug Something isn't working label Sep 23, 2023
@sylikc
Copy link
Owner

sylikc commented Sep 23, 2023

@Yang-z nice catch, that is definitely a bug. Let me write a test case for this and then update the change and push out a new release.

That's a weird bug, thanks for also suggesting the fix

@sylikc
Copy link
Owner

sylikc commented Sep 23, 2023

🤔 I think there could be all types of bugs coming from that. like if UserComment was 0003 or something...

@sylikc
Copy link
Owner

sylikc commented Oct 21, 2023

FYI: I'm writing tests and trying to think of an elegant way to implement this... might just have to be giving users an option. It's very funky behavior actually... I really wish the JSON adhered to the tag type

@sylikc
Copy link
Owner

sylikc commented Oct 21, 2023

So I forsee a problem... I may have to remove this code which supports ujson, since the ujson library does not take in the parameters of the built-in json library

try:
# Optional UltraJSON library - ultra-fast JSON encoder/decoder, drop-in replacement
import ujson as json
JSONDecodeError = ValueError # ujson doesn't throw json.JSONDecodeError, but ValueError when string is malformed
except ImportError:
import json # type: ignore # comment related to https://github.com/python/mypy/issues/1153
from json import JSONDecodeError

Assuming I pull out the json.loads call or make an adjustment to params passed to it, I would have to drop support for ujson to address this issue

Let me think more about how to solve this without breaking too much compatibility... in theory, by removing direct (seamelss) support to ujson, I might be able to effectively enable usage of other libraries like simplejson or orjson in the API

@sylikc sylikc linked a pull request Oct 21, 2023 that will close this issue
@sylikc
Copy link
Owner

sylikc commented Oct 22, 2023

@Yang-z thanks for reporting this bug... I think there's other quirky undocumented behavior with exiftool that might end up being bugs. I wrote an FAQ for set_json_loads that you can use to address this issue

@Yang-z
Copy link
Author

Yang-z commented Oct 27, 2023

@sylikc I'm happy to see you find a way out elegantly. Thanks a lot. I am going to try the new version now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants