[JakeSmarter]

Follow up to #782.

[ptpt]

Is the PR still needed if decimal values are serialized in JSON, instead of converting to EXIF rational values? See my comment in #782.

[JakeSmarter]

It is at least in that sense that the current hard coded decimal fraction precision of 7 places can be insufficient to exactly represent some Exif rationals in the range -100 < x < 100. Furthermore, this PR also enables users who want to consume and modify the image description file to tune precision to their needs.

[ptpt]

@JakeSmarter Do we really need to care about the whole number part? Can we just increase the precision to 8 for all?

[JakeSmarter]

@ptpt We do not have to care about the integer part but then we would need to increase precision to 10 faction decimal places in order to represent full precision for all possible Exif rational values. In fact, by doing so we would be able to represent a lot more values than what is actually needed to fully represent Exif rationals. On average, this is going to result in quite long decimal numbers. Much longer than actually needed in the vast majority of cases. A sign character, 13 digit characters, and a decimal fraction dot character in the worst case scenario. In total, 15 bytes for one number. Is this something we can or want to accept? I have no opinion on this. I am just stating potential consequences. This PR limits emitted numbers to 10 (or fewer if possible) decimal places, which is enough to represent all possible Exif rational values. You could call it an (space) optimization.

By the way, I have already successfully uploaded multiple sequences using this code branch. Again, I have no opinion on this. Maybe someone can disprove my reasoning, which I am also fine with. Do as you like.

[JakeSmarter]

I have changed my mind because I think this PR would make for a nice addition to mapillary_tools. And, I do not think it is going to hurt anybody nor break the backend. You can tune the default precision to your liking.

However, I am not sure what this failing unit test is supposed to test.

tests/unit/test_description.py:135: AssertionError
=========================== short test summary info ============================
FAILED tests/unit/test_description.py::test_encode_base_point - assert 10.1235 == 10.123
 +  where 10.123 = round(10.1235, 3)
======= 1 failed, 362 passed, 12 skipped, 1 xfailed in 100.93s (0:01:40) =======

If anything, I think you may want to rather test the whole range of (useful) orders of precision from 1 to 10 by setting the appropriate environment variables than just some fixed default values.

[caglarpir]

Thanks for following up on #782, @JakeSmarter, for explaining the reasoning in so much detail, and for rebasing this onto 0.15.0. I've gone back and forth on this one, but I'd rather not make number formatting configurable through three environment variables: it's more to document and test, and the description file isn't really meant to be tuned by users. Seven decimal places is already about 1 cm on the ground, so the extra digits don't change where an image ends up. If there's a real case where 7 places (or 3 for altitude and heading) loses something that matters, I'd be open to a small PR that raises the fixed precision, as @ptpt suggested.

[JakeSmarter]

Got it.

My reasoning behind it was just that the conversion between Exif data records to database cells should be lossless, although the additional precision for the Mapillary use case effectively does not matter now. Perhaps someone may want to recreate the exact values in the future to run FFTs over this metadata to trace the evolution of GPS receivers, etc. 😉

Thank you @caglarpir.