DateTime->from_object, DateTime::Format::Epoch, and leap seconds

[esabol]

Hi, I'm using DateTime::Format::Epoch to parse values in seconds since 1980-01-06 00:00:00 UTC. Everything works great unless the value for the time coincides with a leap second. One second less, it works. One second more, it works. For the leap second itself, I get the following error:

Validation failed for type named Hour declared in package DateTime::Types (/usr1/local/perl5/5.28.0/lib/site_perl/5.28.0/x86_64-linux/DateTime/Types.pm) at line 101 in sub named (eval) with value 24

Trace begun at Specio::Exception->new line 57
Specio::Exception::throw('Specio::Exception', 'message', 'Validation failed for type named Hour declared in package DateTime::Types (/usr1/local/perl5/5.28.0/lib/site_perl/5.28.0/x86_64-linux/DateTime/Types.pm) at line 101 in sub named (eval) with value 24', 'type', 'Specio::Constraint::Simple=HASH(0x271c540)', 'value', 24) called at (eval 220) line 144
DateTime::_check_new_params('second', 0, 'hour', 24, 'month', 6, 'day', 30, 'year', 2012, 'nanosecond', 0, 'minute', 0, 'time_zone', 'UTC') called at /usr1/local/perl5/5.28.0/lib/site_perl/5.28.0/x86_64-linux/DateTime.pm line 176
DateTime::new('DateTime', 'second', 0, 'hour', 24, 'month', 6, 'day', 30, 'year', 2012, 'nanosecond', 0, 'minute', 0, 'time_zone', 'UTC') called at /usr1/local/perl5/5.28.0/lib/site_perl/5.28.0/x86_64-linux/DateTime.pm line 611
DateTime::from_object('DateTime', 'object', 'DateTime::Format::Epoch::_DateTime=HASH(0x1a32888)') called at /usr1/local/perl5/5.28.0/lib/site_perl/5.28.0/DateTime/Format/Epoch.pm line 183
DateTime::Format::Epoch::parse_datetime('DateTime::Format::Epoch=HASH(0x2ef4cf8)', 1025136015) called at /path/to/test.pl line 60

Under the hood, DateTime::Format::Epoch is executing:

    my $temp_dt = bless { rd_days => $rd_days, rd_secs => $rd_secs},
                        'DateTime::Format::Epoch::_DateTime';

    my $dt = $self->{epoch_class}->from_object( object => $temp_dt );

In this specific case, $rd_days is 734684 and $rd_secs => 86400 and $self->{epoch_class} is "DateTime". DateTime::LeapSecond::day_length( 734684 ) returns 86401. The DateTime::Format::Epoch::_DateTime class only has one method: utc_rd_values.

Here's my code:

use DateTime;
use DateTime::TimeZone;
use DateTime::Format::Epoch;

my $utc = DateTime::TimeZone->new(name => 'Etc/UTC');

my $gps_epoch = DateTime->new(year => 1980, month => 1, day => 6,
                              hour => 0, minute => 0, second => 0,
                              time_zone => $utc); # 1980-01-06 00:00:00 UTC

my $gps_parser = DateTime::Format::Epoch->new(epoch => $gps_epoch,
                                              dhms => 0,
                                              start_at => 0,
                                              unit => 'seconds',
                                              type => 'float',
                                              skip_leap_seconds => 0);

print "DateTime $DateTime::VERSION\n";
print "DateTime::LeapSecond $DateTime::LeapSecond::VERSION\n";
print "DateTime::Format::Epoch $DateTime::Format::Epoch::VERSION\n";
my $date = '1025136015';
print "date: $date\n";
my $dt = $gps_parser->parse_datetime($date);
print "dt: $dt\n";
print "tz: ",$dt->time_zone,"\n";

Versions:

DateTime 1.52
DateTime::LeapSecond 1.52
DateTime::Format::Epoch 0.16

Thanks!

[esabol]

In this specific case, $rd_days is 734684 and $rd_secs => 86400 and $self->{epoch_class} is "DateTime". DateTime::LeapSecond::day_length( 734684 ) returns 86401. The DateTime::Format::Epoch::_DateTime class only has one method: utc_rd_values.

So I think that's the problem. When dealing with leap seconds, from_object checks to make sure the time zone isn't floating, and DateTime::Format::Epoch::_DateTime doesn't have a time_zone method.

Adding a time_zone method to DateTime::Format::Epoch::_DateTime fixes the problem in my testing, but I wonder if DateTime might be a bit more liberal here? Maybe change

        if (   $object->can('time_zone')
            && !$object->time_zone->is_floating
            && $rd_secs > 86399
            && $rd_secs <= $class->_day_length($rd_days) ) {
            $leap_seconds = $rd_secs - 86399;
            $rd_secs -= $leap_seconds;
        }

to

        if (   (($object->can('time_zone')
                  && !$object->time_zone->is_floating)
                || !$object->can('time_zone'))
            && $rd_secs > 86399
            && $rd_secs <= $class->_day_length($rd_days) ) {
            $leap_seconds = $rd_secs - 86399;
            $rd_secs -= $leap_seconds;
        }

Basically, if the object doesn't have a time_zone method, assume the time_zone is not floating. What do you think?

There hasn't been a DateTime::Format::Epoch release in 4 years, so getting it fixed over there seems like a tough road....

[n1vux]

There hasn't been a DateTime::Format::Epoch release in 4 years, so getting it fixed over there seems like a tough road....

Without checking the severity of their 7 open issues, i wouldn't presume one way or the other.

If there is an obvious and simple fix to allow that module to be leapsecond safe, if any of the three "maintainers" are still involved at all, they should accept a pull request from a fork.
(And even if they reject it, you've got a working fork.)

OTOH if the listed three are not in fact still involved, the module could be listed as orphaned and put up for adoption.

(You should want to know if there's any support if you're using this module for anything of value. If it's unsupported, either take on support of it, or get someone else to, or use a different module to do the same job.)

Basically, if the object doesn't have a time_zone method, assume the time_zone is not floating. What do you think?

(I'm assuming you mean a time_zone value in it's (key, value) attributes when you say method.)

I think "floating" is the least unsafe assumption if a time-zone is not specified.

Lack of a specified timezone is innately ambiguous (Z=UTC? User's graphic desktop Locale?) and highly dangerous when dealing with any corner-case semantics (DST, timezones, day of week, leapseconds, ...) and should be considered erroneous.

(If one wants to represent Noon in whatever timezone it is later evaluated in, DateTime is the wrong object; "12:00" as a string is fine, use it to make a DateTime when both the Date and Timezone it's to be combined with are known.)

Without a specification of TZ it's impossible to validate if a leap second is valid or not (valid at UTC midnight on a very short list of days, possibly finite list if mooted reforms are enacted).
So it would be unsafe to specify 2012-06-30T23:59:60 as a floating time -- it should error.
(It gave you the wrong error, complaining of hour 24 instead of seconds 60, and that's bad too. But if it was either floating or no-timezone such that it was not unambiguously a legit leapsecond, normalizing by carrying the 60 is fair; i'm surprised it didn't carry the second 24 and increment to July 1st 00:00:00 !)
If the Epoch module doesn't parse your leapsecond into a UTC DateTime or whatever TZ you've requested, it's in need of fixing.

[esabol]

Basically, if the object doesn't have a time_zone method, assume the time_zone is not floating. What do you think?

(I'm assuming you mean a time_zone value in it's (key, value) attributes when you say method.)

No, I meant “method”. The code checks that the object can('time_zone'). That means method. (Did you look at my code snippet?)

I think "floating" is the least unsafe assumption if a time-zone is not specified.

Not applicable here! The date/time is specified as UTC Rata Die days, seconds, and nanoseconds. UTC is right in the name. The timezone is inherently UTC.

If the Epoch module doesn't parse your leapsecond into a UTC DateTime or whatever TZ you've requested, it's in need of fixing.

That’s not what’s happening here at all. I feel like you aren’t understanding the issue. Maybe I did a lousy job of explaining it?

[ikegami]

DT::F::Epoch wants to create an object from rd data (rd_days and rd_secs). It does so by creating an DT::F::Epoch::_DateTime (which isn't a sub class of DateTime) and passes this object to a method of DateTime that expects a DateTime object.

DT::F::Epoch is clearly at fault here, and I don't think DateTime should humour DT::F::Epoch, at least not in the way suggested by the OP. What might make sense is for DateTime to provide a way of constructing an object from rd data.

[ikegami]

In the absence of a sanctioned means to create a DateTime object from rd data, I think the problem can be fixed using

my $utc = DateTime::TimeZone->new( name => 'Etc/UTC' );
sub DateTime::Format::Epoch::_DateTime::time_zone { $utc }

[autarch]

The DateTime->from_object constructor call ->utc_rd_values on the object it's passed.

[ikegami]

The DateTime->from_object constructor call ->utc_rd_values on the object it's passed.

That's fine. DT::F::Epoch::_DateTime provides that method.

The problem is that it a call to ->time_zone was added, and DT::F::Epoch::_DateTime doesn't provide that one.

[esabol]

I still don’t see why it isn’t reasonable to assume UTC Rata Die values are actually UTC, but whatever. I’d be happy to see this fixed in DateTime::Format::Epoch, but, after 2.5 months, the maintainer of that package isn’t responding to my GitHub issue: chorny/DateTime-Format-Epoch#3

@autarch: I think I saw you have a CPAN maintainer bit for DateTime::Format::Epoch. Would you consider forking it into houseabsolute and releasing an update? I’d be happy to submit a PR.

[ikegami]

I still don’t see why it isn’t reasonable to assume UTC Rata Die values are actually UTC

That's not what I said. I said it's not appropriate to add hacks to DateTime because some module is using it incorrectly.

the maintainer of that package isn’t responding to my GitHub issue:

I fail to see how that's relevant.

[esabol]

the maintainer of that package isn’t responding to my GitHub issue:

I fail to see how that's relevant.

It’s relevant to the next two sentences (which are addressed to @autarch).

[autarch]

@autarch: I think I saw you have a CPAN maintainer bit for DateTime::Format::Epoch. Would you consider forking it into houseabsolute and releasing an update? I’d be happy to submit a PR.

Let me make one last attempt to contact the maintainer first. If they don't respond, yes, I'm up for this.

[n1vux]

Let me make one last attempt to contact the maintainer first.

Right and proper.

If they don't respond, yes, I'm up for this.

Highly likely either no response or if their CPAN email still forwards to a valid mailbox they'll happily gift it. DT-F-E hasn't been maintained in 5 years. Of the two with CPAN Authorized Release bits, CHORNY hasn't released anything in 3 years, PIJLL in 13 years.