Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: daf_base
-
Labels:
-
Story Points:2
-
Epic Link:
-
Sprint:Alert Production F16 - 9
-
Team:Alert Production
Description
The following fails for reasons I do not understand:
from lsst.daf.base import DateTime
|
# for most versions of daf_base
|
DateTime("1969-12-31T23:59:59Z")
|
# if using the latest master of daf_base
|
DateTime("1969-12-31T23:59:59Z", DateTime.UTC)
|
The error is:
File "src/DateTime.cc", line 391, in lsst::daf::base::DateTime::DateTime(int, int, int, int, int, int, lsst::daf::base::DateTime::Timescale)
|
Unconvertible date: 1969-12-31T23:59:59 {0}
|
lsst::pex::exceptions::DomainError: 'Unconvertible date: 1969-12-31T23:59:59'
|
and comes from this code:
dafBase::DateTime::DateTime(int year, int month, int day,
|
int hr, int min, int sec, Timescale scale) {
|
|
|
|
|
struct tm tm;
|
tm.tm_year = year - 1900;
|
tm.tm_mon = month - 1;
|
tm.tm_mday = day;
|
tm.tm_hour = hr;
|
tm.tm_min = min;
|
tm.tm_sec = sec;
|
tm.tm_wday = 0;
|
tm.tm_yday = 0;
|
tm.tm_isdst = 0;
|
tm.tm_gmtoff = 0;
|
|
|
time_t secs = timegm(&tm);
|
|
// long long nsecs will blow out beyond sep 21, 1677 0:00:00, and apr 12 2262 00:00:00
|
// (refering to the values of EPOCH_IN_MJD +/- MAX_DAYS ... exceeds 64 bits.)
|
// However, a tm struct is only 32 bits, and saturates at:
|
// low end - Dec 13 1901, 20:45:52
|
// hi end - Jan 19 2038, 03:14:07
|
|
if (secs == -1) {
|
throw LSST_EXCEPT(
|
lsst::pex::exceptions::DomainError,
|
(boost::format("Unconvertible date: %04d-%02d-%02dT%02d:%02d:%02d")
|
% year % month % day % hr % min % sec).str());
|
}
|
Attachments
Activity
Fair enough, Russell Owen, I agree with you.
Paul Price
added a comment - Fair enough, Russell Owen , I agree with you. It turns out DateTime would wrap dates around (different ISO date strings could result in the same date) due to limitations in timegm. In order to prevent this I added a test to the constructors that use timegm (including the ISO string constructor) to reject dates with year < 1902 or > 2261. This is not the precise limits, but is simple and close enough. This can still wrap on 32-bit systems, so I added a unit test to detect that. The test will detect the problem and prevent scons from successfully building the package on such systems.
It is still possible to construct a wider range of dates by using the constructor that takes nanoseconds, and these are correctly displayed, since they don't use timegm. I don't have a strong opinion on whether this is a feature or a misfeature, but even if the latter, I suggest we live with it.
Also, it turns out that DateTime._str and repr_ has a misfeature: they used UTC, which meant that they raised an exception on instances of DateTime before the start of the leap second table (some time in 1961). I fixed this by changing them to display in TAI. To make the resulting output clear I changed the format to include "TAI", and at that point it was verbose enough that there was no point in making the two different, plus it gave me a chance to fix the issue Paul Price pointed out. The new format is DateTime("<iso_string>", TAI).
Paul Price and/or Kian-Tat Lim: would you be willing to take another look at the code?
Review completed on GitHub.
Paul Price
added a comment - Review completed on GitHub. Thank you for the helpful review. I checked the issue about year for unix second = -1 (fortunately it is not a problem) and made all suggested changes.
A mutex would only work if we can guarantee that this is the only code in the entire system that uses the TZ environment variable or the tzname global variable, which seems difficult. See also http://www.catb.org/esr/time-programming/#_good_programming_practice. All of this is moot once we integrate with astropy.time.