Unit test test_issue_1959.py fails when run on a system whose UTC offset is nonzero #2083
Labels
Comments
|
I've submitted PR #2084 to address this issue. |
|
@mallman Can you run the test as follows and send me your output please. Here's what I get in England (GMT, UTC): |
|
Here you go! |
|
Michael @mallman Thanks for sharing this with me. I don't know why I'm only seeing this email today on Sunday when you posted it on Thursday. I know I've spoken to you elsewhere about this when I remembered how to "go home" to California. The issue is being caused by the metadata convertors. I think the aim of the test is to read sidecar files (.xmp). Choosing a side-car file which doesn't have timestamps will probably evade this part of the convertor and the test will no longer be sensitive to the host TZ settings. @postscript-dev may have discovered a way in which to run all tests in TZ=UTC. For sure this is ugly and undesirable. I'm confident that we can make this disappear with a change in the test suite. We need a better understanding of the rationale for the convertors. Perhaps the library needs a global switch to enable/disable metadata convertors. |
|
This looks mostly pain free. There's a comment in src/xmpsidecar.cpp That refers to 1112 on redmine. So this was about 2014 or so. The 1112 issue was that the XMP to Exif convertor rounded the XMP (XML date with TZ and decimal point seconds) to Exif seconds (which at that time didn't support sub-seconds, or TZ offsets). Then the Exif to XMP convertor copied the Exif DataTime into the XMP. So my fix in 1112 was to store the XML date before the convertor dance and restore them in writeMetadata(). Here are examples of the date time formats in Exif and XML. You'll spot the image was taken in July when the UK was in Summer Time (+1.00). I labelled the image "Classic View" about 8:30pm when I got home from taking guests to visit the stones and Picasa correctly set So, we have the XML date in our hands and have to add something like: std::string dateTimeOriginal = 'Xmp.photoshop.DateCreated' ;
if ( exifData_.find("Exif.Photos.DateTimeOriginal") >= 0 && dates_.count(dateTimeOriginal) ) {
exifData_["Exif.Photo.DateTimeOriginal"] = xmlToExif(dates_[dateTimeOriginal]);
}We'll need an xmlToExif() convertor which is almost certain in the code base. We might need to tell him to ignore TZ. 542 rmills@rmillsmm-local:~/gnu/github/exiv2/main $ grep date src/convert.cpp
@brief Exif date to XMP conversion function.
Sets the XMP property to an XmpText value containing date and time. This function
@brief XMP to Exif date conversion function.
Converts the XmpText value to Exif date and time. This function
XMP and Exif was updated more recently and copies metadata in appropriate direction.
...This looks straight-forward to fix. |
|
Hi @clanmills. I don't think that an image with an embedded The problem is we don't have The problem isn't a lack of a conversion function or a bug in a conversion function. I think the problem is such a conversion function is impossible without timezone information. So one solution to this problem is specifying a local time zone or UTC offset as a parameter of a "timestamp to local time" function. I think this is the essence of how and why exiv uses the system time zone or the Likewise, I suggest a function that converts a local time to a timestamp, should require an additional time zone (or offset) function parameter to specify the time zone of the local time. |
|
@mallman I think there are two different subjects here. The first and immediate puzzle is to get the #1959 test to pass. The test in #1959 arrived with a request from IPTC to provide support for changes to their standards. I've outlined an approach that I believe will result in #1959 passing without causing issues to existing tests. The second puzzle is the specification for the convertors. Are they based on work by a standards committee? I don't know. I don't want to get further drawn into this matter. I retired last year from Exiv2 (after 13 years). I'm continuing to support the 0.27-maintenance branch and made two releases in 2021. I expect another "dot" will be required in 2022 as my final contribution to the project. |
Done! (Indirectly, but done nonetheless)
Let's take this out of scope. I thought—naively—that this time conversion issue could be contained to a tidy scope. Wrong. Moving on to more higher priorities... |
clanmills
added a commit
that referenced
this issue
Feb 16, 2022
…ring it, adding to test_reference_files and evading the TZ issue discussed in #2083.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Unit test test_issue_1959.py fails when run on a system whose UTC offset is nonzero. For example, this test fails on my system, where the time zone is set to Pacific Standard Time. However, the failure is simply a result of the test expecting
exiv2to run with a UTC offset of zero.To Reproduce
Steps to reproduce the behavior:
Run ctest on a machine with a time zone which has a nonzero offset from UTC.
Expected behavior
Test test_issue_1959 passes.
Desktop (please complete the following information):
Additional context
While I don't know if this is the "best" or "correct" solution to this issue, I was able to fix it by modifying the command in
exiv2/tests/bugfixes/github/test_issue_1959.py
Line 18 in 8505f4d
to
Unless I hear otherwise, I will submit a PR with this change. Cheers.
The text was updated successfully, but these errors were encountered: