chronotope / chrono

Date and time library for Rust
Other
3.33k stars 532 forks source link

Easy way to check a time is a leap second #49

Open alex opened 9 years ago

alex commented 9 years ago
extern crate chrono;

use chrono::{UTC, TimeZone, Timelike};

fn main() {
    let s = "100102030460Z";
    let d = UTC.datetime_from_str(s, "%y%m%d%H%M%SZ").unwrap();
    println!("{} -- second={}, nanosecond={}", d, d.second(), d.nanosecond());
}
/t/te (master) $ cargo run
     Running `target/debug/te`
2010-01-02 03:04:60 UTC -- second=59, nanosecond=1000000000

Specifically: the string representation of the datetime shows it (correctly) as :60 seconds, however the internal representation stores it as 59 seconds and 1000000000 nanoseconds. In general, smaller units are converted to larger ones as soon as an integral divisor is reached, so I'm not sure why it's not d.second() -> 60 and d.nanosecond() -> 0.

lifthrasiir commented 9 years ago

This is intentional and (while not yet in the currently released version) documented. Can you clarify what is this issue about? I would like to close this issue if your concern is simply a missing documentation.

The core rationale is that you can assume that there is no such thing as a leap second; the exception is when you actually get a leap second (:60 in the formatted string or >=1 fractional seconds), in which case it behaves like an extra fractional second following the original second. Only when you want to check if it were actually a leap second, you can look at the excess fractional seconds or (in the future) convert from the leap second to the linear scale like TAI and vice versa.

alex commented 9 years ago

The undelrying issue I am experiencing is this: I am parsing ASN.1 UTCTime values, which have the format you see in my original example, except they don't allow leap seconds (60 in the second value), while chrono's %S format does. I then did the natural thing, which was to write .second() >= 60 and error out in that case, however, obviously that case is never encountered, because the data is in nanoseconds().

So, I suppose I have two questions/comments:

  1. The fact that Display showed :60 but .seconds() wasn't 60 was confusing.
  2. Is there some other way I should reject values for which %S parses 60 that's better than d.second() >= 59 && d.nanosecond() > 0?
lifthrasiir commented 9 years ago
  1. I feel this is basically a documentation problem, which I'd like to resolve. I already mentioned that Timelike::second always returns a number less than 60 and Timelike::nanosecond would represent a leap second, but any elaboration would be welcoming (I planned to do so eventually).
  2. d.nanosecond() >= 1_000_000_000 should be sufficient to detect a leap second. Would a method Timelike::is_leap_second be good to have? Having a magic number is not very good but I had no concrete example for that method until now.
alex commented 9 years ago

is_leap_second() would be a great solution, it'd be more readable and let me delete several lines of comments :-)

pitdicker commented 7 months ago

Renamed this issue.

Adding something like Timelike::is_leap_second seems easy?

djc commented 7 months ago

@alex -- it's been a while. Is this still of interest to you?

alex commented 7 months ago

I'm no longer using Chrono for this application, so it doesn't impact me directly.

I do think it'd be valuable to have an is leap second API though!