to_std is lossy

[elbaro]

let d = ...::parse("PT1H").unwrap();
println!("{:?}\n{:?}", d, d.to_std());

Duration { year: 0.0, month: 0.0, day: 0.0, hour: 1.0, minute: 0.0, second: 0.0 }
3599.99995904s

[rrichardson]

@elbaro - It might be a bit late, but I put in a PR to fix the to_std() issue.
I can't guarantee that it won't ever happen, because floating point values are somewhat probabilistic, but it shouldn't happen under normal circumstances.

[elbaro]

Didn't know that iso8601 allows fractions in the place of hours or minutes.

Now I see how difficult this is - a small floating error in the number of days becomes non-negligible when it's multiplied by 24*60*60*1e9 ns/day. Supporting common use cases would be fine.

[FrederickFrance]

Great Same problem I hope your PR will be merged

[FrederickFrance]

@rrichardson I need nom 7.0.0 to use tarpaulin, I sent you a PR

[rrichardson]

A PR for my PR. Awesome :) I'll merge it, but I was just thinking about @PoiScript's comments in https://github.com/PoiScript/iso8601-duration/pull/5. They are correct, there isn't a way to accurately apply a duration to a Date without using knowledge of a calendar. My implementation is just a rough guess based on the average length of a month.

With that in mind, what I think this module should be is a trait that we impl for common Date implementations like Chrono NaiveDate etc.

e.g.

use chrono::{ NaiveDate, Duration as ChronoDuration};
use crate::duration::Duration;

trait IsoDurationExt {
  fn parse_from_str(&str) -> Result<Self, Error>,
}

impl IsoDurationExt for ChronoDuration {
  fn parse_from_str(data: &str) -> Result<Self, IsoParseError> {
      Duration::parse(data).map(|d| {
        ChronoDuration::years(d.year.round() as i64)
        + ChronoDuration::months(d.month.round() as i64) 
        + ChronoDuration::days(d.day.round() as i64) 
        + ChronoDuration::hours(d.hour.round() as i64) 
        + ChronoDuration::minutes(d.minute.round() as i64) 
        + ChronoDuration::seconds(d.second.round() as i64) 
        + ChronoDuration::millis(d.milli.round() as i64)
      })
  } 
}  

impl<T> FromStr for T where T: IsoDurationExt {
  type Err = IsoParseError;
  fn from_str(s: &str): -> Result<Self, Self::Err> {
    T::parse_from_str(s)
  }
}

Then all the consumer would have to do is :

let start = NaiveDate::from_ymd(1970, 1, 1).and_hms(0, 0, 0, 0);

let end = start + ChronoDuration::from_str("PT1H").unwrap(); 

[rrichardson]

Actually.. I just noticed that Chrono::Duration punts on the month problem.. it only lets you specify a duration in increments of weeks or smaller

[rrichardson]

The most widely used Duration parsing system that I know of is the one in Golang.. They, too, don't handle any increment larger than an hour. https://pkg.go.dev/time

[rrichardson]

I'm starting to think that for practical applications, we should just ban the use of Month for anything other than parsing.

Conversion to any duration that is based on seconds is going to result in sadness.

We would have to rely on something like chrono to do the math for us, e.g. in order to properly create a duration that includes months, we'd have to manually construct 2 dates and subtract the difference.

[rrichardson]

Actually.. based on the discussions I'me seeing in Chrono and time, we should probably just point it to the time:0.3 crate.

[FrederickFrance]

A PR for my PR. Awesome :) I'll merge it, but I was just thinking about @PoiScript's comments in #5. They are correct, there isn't a way to accurately apply a duration to a Date without using knowledge of a calendar. My implementation is just a rough guess based on the average length of a month.

With that in mind, what I think this module should be is a trait that we impl for common Date implementations like Chrono NaiveDate etc.

e.g.

use chrono::{ NaiveDate, Duration as ChronoDuration};
use crate::duration::Duration;

trait IsoDurationExt {
  fn parse_from_str(&str) -> Result<Self, Error>,
}

impl IsoDurationExt for ChronoDuration {
  fn parse_from_str(data: &str) -> Result<Self, IsoParseError> {
      Duration::parse(data).map(|d| {
        ChronoDuration::years(d.year.round() as i64)
        + ChronoDuration::months(d.month.round() as i64) 
        + ChronoDuration::days(d.day.round() as i64) 
        + ChronoDuration::hours(d.hour.round() as i64) 
        + ChronoDuration::minutes(d.minute.round() as i64) 
        + ChronoDuration::seconds(d.second.round() as i64) 
        + ChronoDuration::millis(d.milli.round() as i64)
      })
  } 
}  

impl<T> FromStr for T where T: IsoDurationExt {
  type Err = IsoParseError;
  fn from_str(s: &str): -> Result<Self, Self::Err> {
    T::parse_from_str(s)
  }
}

Then all the consumer would have to do is :

let start = NaiveDate::from_ymd(1970, 1, 1).and_hms(0, 0, 0, 0);

let end = start + ChronoDuration::from_str("PT1H").unwrap(); 

For me it was ok because I use only hours, minutes and seconds ;)

[PoiScript]

Sorry for the delayed response! I just released a 0.2 version to solve this long-exist problem:

1. Duraion::to_std now returns a None when year or month is used.
2. To get an accurate duration, you can enable chrono feature and specify a staring date by call Duration::to_chrono_at_datetime:

use iso8601_duration::Duration;
use chrono::DateTime;

let one_month: Duration = "P1M".parse().unwrap();
let date = DateTime::parse_from_rfc3339("2000-02-01T00:00:00Z").unwrap();
assert_eq!(
    one_month.to_chrono_at_datetime(date).num_days(),
    29 // 2000 is a leap year
);