Merge pull request #273 from nyx-space/272-returns-wrong-day-of-year

[Maybe breaking change] Fix day of year computation

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "hifitime"
-version = "3.8.7"
+version = "3.9.0"
 authors = ["Christopher Rabotin <[email protected]>"]
 description = "Ultra-precise date and time handling in Rust for scientific applications with leap second support"
 homepage = "https://nyxspace.com/"

src/duration.rs

@@ -1008,6 +1008,7 @@ impl Add for Duration {
     /// ## Examples
     /// + `Duration { centuries: 0, nanoseconds: 1 }` is a positive duration of zero centuries and one nanosecond.
     /// + `Duration { centuries: -1, nanoseconds: 1 }` is a negative duration representing "one century before zero minus one nanosecond"
+    #[allow(clippy::absurd_extreme_comparisons)]
     fn add(self, rhs: Self) -> Duration {
         // Check that the addition fits in an i16
         let mut me = self;

src/efmt/format.rs

@@ -90,7 +90,7 @@ const MAX_TOKENS: usize = 16;
 /// assert_eq!(fmt, consts::ISO8601_ORDINAL);
 ///
 /// let fmt_iso_ord = Formatter::new(bday, consts::ISO8601_ORDINAL);
-/// assert_eq!(format!("{fmt_iso_ord}"), "2000-059");
+/// assert_eq!(format!("{fmt_iso_ord}"), "2000-060");
 ///
 /// let fmt = Format::from_str("%A, %d %B %Y %H:%M:%S").unwrap();
 /// assert_eq!(fmt, consts::RFC2822_LONG);
@@ -549,7 +549,16 @@ fn epoch_format_from_str() {
 #[cfg(feature = "std")]
 #[test]
 fn gh_248_regression() {
+    /*
+    Update on 2023-12-30 to match the Python behavior:
+
+    >>> from datetime import datetime
+    >>> dt, fmt = "2023-117T12:55:26", "%Y-%jT%H:%M:%S"
+    >>> datetime.strptime(dt, fmt)
+    datetime.datetime(2023, 4, 27, 12, 55, 26)
+     */
+
     let e = Epoch::from_format_str("2023-117T12:55:26", "%Y-%jT%H:%M:%S").unwrap();
 
-    assert_eq!(format!("{e}"), "2023-04-28T12:55:26 UTC");
+    assert_eq!(format!("{e}"), "2023-04-27T12:55:26 UTC");
 }

src/epoch.rs

@@ -1232,9 +1232,14 @@ impl Epoch {
     /// # Limitations
     /// In the TDB or ET time scales, there may be an error of up to 750 nanoseconds when initializing an Epoch this way.
     /// This is because we first initialize the epoch in Gregorian scale and then apply the TDB/ET offset, but that offset actually depends on the precise time.
+    ///
+    /// # Day couting behavior
+    ///
+    /// The day counter starts at 01, in other words, 01 January is day 1 of the counter, as per the GPS specificiations.
+    ///
     pub fn from_day_of_year(year: i32, days: f64, time_scale: TimeScale) -> Self {
         let start_of_year = Self::from_gregorian(year, 1, 1, 0, 0, 0, 0, time_scale);
-        start_of_year + days * Unit::Day
+        start_of_year + (days - 1.0) * Unit::Day
     }
 }
 
@@ -2486,24 +2491,26 @@ impl Epoch {
     #[must_use]
     /// Returns the duration since the start of the year
     pub fn duration_in_year(&self) -> Duration {
-        let year = Self::compute_gregorian(self.to_duration()).0;
-        let start_of_year = Self::from_gregorian(year, 1, 1, 0, 0, 0, 0, self.time_scale);
+        let start_of_year = Self::from_gregorian(self.year(), 1, 1, 0, 0, 0, 0, self.time_scale);
         self.to_duration() - start_of_year.to_duration()
     }
 
     #[must_use]
     /// Returns the number of days since the start of the year.
     pub fn day_of_year(&self) -> f64 {
-        self.duration_in_year().to_unit(Unit::Day)
+        self.duration_in_year().to_unit(Unit::Day) + 1.0
+    }
+
+    #[must_use]
+    /// Returns the number of Gregorian years of this epoch in the current time scale.
+    pub fn year(&self) -> i32 {
+        Self::compute_gregorian(self.duration_since_j1900_tai).0
     }
 
     #[must_use]
     /// Returns the year and the days in the year so far (days of year).
     pub fn year_days_of_year(&self) -> (i32, f64) {
-        (
-            Self::compute_gregorian(self.to_duration()).0,
-            self.day_of_year(),
-        )
+        (self.year(), self.day_of_year())
     }
 
     /// Returns the hours of the Gregorian representation  of this epoch in the time scale it was initialized in.

tests/epoch.rs

@@ -3,17 +3,14 @@ extern crate core;
 
 use hifitime::{
     is_gregorian_valid, Duration, Epoch, Errors, ParsingErrors, TimeScale, TimeUnits, Unit,
-    Weekday, BDT_REF_EPOCH, DAYS_GPS_TAI_OFFSET, GPST_REF_EPOCH, GST_REF_EPOCH, J1900_OFFSET,
-    J1900_REF_EPOCH, J2000_OFFSET, MJD_OFFSET, SECONDS_BDT_TAI_OFFSET, SECONDS_GPS_TAI_OFFSET,
-    SECONDS_GST_TAI_OFFSET, SECONDS_PER_DAY,
+    Weekday, BDT_REF_EPOCH, DAYS_GPS_TAI_OFFSET, DAYS_PER_YEAR, GPST_REF_EPOCH, GST_REF_EPOCH,
+    J1900_OFFSET, J1900_REF_EPOCH, J2000_OFFSET, MJD_OFFSET, SECONDS_BDT_TAI_OFFSET,
+    SECONDS_GPS_TAI_OFFSET, SECONDS_GST_TAI_OFFSET, SECONDS_PER_DAY,
 };
 
 use hifitime::efmt::{Format, Formatter};
 
-#[cfg(feature = "std")]
 use core::f64::EPSILON;
-#[cfg(not(feature = "std"))]
-use std::f64::EPSILON;
 
 #[test]
 fn test_const_ops() {
@@ -1043,7 +1040,7 @@ fn test_leap_seconds_iers() {
     let epoch_from_utc_greg = Epoch::from_gregorian_tai_hms(1971, 12, 31, 23, 59, 59);
     // Just after it.
     let epoch_from_utc_greg1 = Epoch::from_gregorian_tai_hms(1972, 1, 1, 0, 0, 0);
-    assert_eq!(epoch_from_utc_greg1.day_of_year(), 0.0);
+    assert_eq!(epoch_from_utc_greg1.day_of_year(), 1.0);
     assert_eq!(epoch_from_utc_greg.leap_seconds_iers(), 0);
     // The first leap second is special; it adds 10 seconds.
     assert_eq!(epoch_from_utc_greg1.leap_seconds_iers(), 10);
@@ -1777,10 +1774,10 @@ fn test_epoch_formatter() {
     let bday = Epoch::from_gregorian_utc(2000, 2, 29, 14, 57, 29, 37);
 
     let fmt_iso_ord = Formatter::new(bday, ISO8601_ORDINAL);
-    assert_eq!(format!("{fmt_iso_ord}"), "2000-059");
+    assert_eq!(format!("{fmt_iso_ord}"), "2000-060");
 
     let fmt_iso_ord = Formatter::new(bday, Format::from_str("%j").unwrap());
-    assert_eq!(format!("{fmt_iso_ord}"), "059");
+    assert_eq!(format!("{fmt_iso_ord}"), "060");
 
     let fmt_iso = Formatter::new(bday, ISO8601);
     assert_eq!(format!("{fmt_iso}"), format!("{bday}"));
@@ -1854,7 +1851,6 @@ fn test_leap_seconds_file() {
 #[test]
 fn regression_test_gh_204() {
     use core::str::FromStr;
-    use hifitime::Epoch;
 
     let e1700 = Epoch::from_str("1700-01-01T00:00:00 TAI").unwrap();
     assert_eq!(format!("{e1700:x}"), "1700-01-01T00:00:00 TAI");
@@ -1871,3 +1867,43 @@ fn regression_test_gh_204() {
     let e1900_m1 = Epoch::from_str("1899-12-31T23:59:59 TAI").unwrap();
     assert_eq!(format!("{e1900_m1:x}"), "1899-12-31T23:59:59 TAI");
 }
+
+#[test]
+fn regression_test_gh_272() {
+    use core::str::FromStr;
+
+    let epoch = Epoch::from_str("2021-12-21T00:00:00 GPST").unwrap();
+
+    let (years, day_of_year) = epoch.year_days_of_year();
+
+    assert!(dbg!(day_of_year) < DAYS_PER_YEAR);
+    assert!(day_of_year > 0.0);
+    assert_eq!(day_of_year, 355.0);
+
+    assert_eq!(years, 2021);
+
+    // Check that even in GPST, we start counting the days at one, in all timescales.
+    for ts in [
+        TimeScale::TAI,
+        TimeScale::GPST,
+        TimeScale::UTC,
+        TimeScale::GST,
+        TimeScale::BDT,
+    ] {
+        let epoch = Epoch::from_gregorian_at_midnight(2021, 12, 31, ts);
+        let (years, day_of_year) = epoch.year_days_of_year();
+        assert_eq!(years, 2021);
+        assert_eq!(day_of_year, 365.0);
+
+        let epoch = Epoch::from_gregorian_at_midnight(2020, 12, 31, ts);
+        let (years, day_of_year) = epoch.year_days_of_year();
+        assert_eq!(years, 2020);
+        // 366 days in 2020, leap year.
+        assert_eq!(day_of_year, 366.0);
+
+        let epoch = Epoch::from_gregorian_at_midnight(2021, 1, 1, ts);
+        let (years, day_of_year) = epoch.year_days_of_year();
+        assert_eq!(years, 2021);
+        assert_eq!(day_of_year, 1.0);
+    }
+}