-
Notifications
You must be signed in to change notification settings - Fork 288
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
fix_1959_tz #2103
fix_1959_tz #2103
Conversation
…ring it, adding to test_reference_files and evading the TZ issue discussed in #2083.
Codecov Report
@@ Coverage Diff @@
## main #2103 +/- ##
=======================================
Coverage 63.75% 63.75%
=======================================
Files 96 96
Lines 19202 19202
Branches 9798 9798
=======================================
Hits 12242 12242
Misses 4658 4658
Partials 2302 2302 Continue to review full report at Codecov.
|
@@ -33,7 +33,6 @@ | |||
exif:ExifVersion="0232" | |||
exif:FlashpixVersion="0100" | |||
exif:ColorSpace="65535" | |||
photoshop:DateCreated="2022-01-04T09:41:01+00:00" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, you took the same approach I wanted to take here: #2079.
For me it is fine to recover the test case like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great minds think alike!
it's not really a fix. It's evasion. It's the metadata convertors. I don't know who thought of adding convertors to the code base. If those convertors are not defined in a standard, they should be implemented in another different project which might use libexiv2. However, I don't know why this is in the metadata engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of test_issue_1959.py
was discussed and agreed on in #2091, based on that it would only duplicate the output reference file.
test/data/issue_1959_poc.xmp.out
is now basically a somewhat less verbose copy of test/data/test_reference_files/issue_1959_poc.xmp.out
That said, I don't have a strong opinion and am happy to go with whatever the crew decides.
But what would be the benefit of adding this test twice?
I don't know why test_issue_1959.py was removed in b318b9d. I'm restoring it, adding to test_reference_files and evading the TZ issue discussed in #2083.
@postscript-dev 1959 was provided by IPTC in #1959. I don't think they will object to this change, however it would be courteous to let the user know that their file has been modified.