Move tz_offset to Time (#32)

* Move tz_offset to Time * update * fix comments * Add comparison methods * Update src/time.rs Co-authored-by: Samuel Colvin <s@muelcolvin.com> * Update src/time.rs Co-authored-by: Samuel Colvin <s@muelcolvin.com> * Fix lint --------- Co-authored-by: Samuel Colvin <s@muelcolvin.com>
pydantic · Apr 7, 2023 · 4edf7b7 · 4edf7b7
1 parent 26b3e31
commit 4edf7b7
Show file tree

Hide file tree

Showing 9 changed files with 347 additions and 190 deletions.
diff --git a/README.md b/README.md
@@ -57,8 +57,8 @@ fn main() {
                 minute: 13,
                 second: 14,
                 microsecond: 0,
+                tz_offset: Some(0),
             },
-            offset: Some(0),
         }
     );
     assert_eq!(dt.to_string(), "2022-01-01T12:13:14Z");

diff --git a/benches/main.rs b/benches/main.rs
@@ -221,6 +221,7 @@ fn format_time(bench: &mut Bencher) {
         minute: 11,
         second: 12,
         microsecond: 11,
+        tz_offset: None,
     });
     bench.iter(|| {
         black_box(time.to_string());
@@ -240,8 +241,8 @@ fn format_date_time(bench: &mut Bencher) {
             minute: 0,
             second: 0,
             microsecond: 0,
+            tz_offset: Some(60),
         },
-        offset: Some(60),
     });
     bench.iter(|| {
         black_box(date.to_string());

diff --git a/fuzz/fuzz_targets/datetime_from_timestamp.rs b/fuzz/fuzz_targets/datetime_from_timestamp.rs
@@ -46,8 +46,8 @@ fn check_timestamp(timestamp: i64, microseconds: u32) {
                             minute: chrono_dt.minute() as u8,
                             second: chrono_dt.second() as u8,
                             microsecond,
+                            tz_offset: None,
                         },
-                        offset: None,
                     },
                     "timestamp: ({}, {}) => speedate({}) != chrono({})",
                     timestamp,

diff --git a/fuzz/fuzz_targets/datetime_parse_bytes.rs b/fuzz/fuzz_targets/datetime_parse_bytes.rs
@@ -20,8 +20,8 @@ fuzz_target!(|data: &[u8]| {
                             minute: chrono_dt.minute() as u8,
                             second: chrono_dt.second() as u8,
                             microsecond: chrono_dt.nanosecond() as u32 / 1_000,
+                            tz_offset: Some((chrono_dt.offset().local_minus_utc() / 60) as i16),
                         },
-                        offset: Some((chrono_dt.offset().local_minus_utc() / 60) as i16),
                     },
                     "timestamp: {:?} => speedate({}) != chrono({})",
                     s,

diff --git a/src/date.rs b/src/date.rs
@@ -162,7 +162,7 @@ impl Date {
     ///
     /// # Arguments
     ///
-    /// * `offset` - timezone offset in seconds, meaning as per [DateTime::now], must be less than `86_400`
+    /// * `tz_offset` - timezone offset in seconds, meaning as per [DateTime::now], must be less than `86_400`
     ///
     /// # Example
     ///
@@ -172,8 +172,8 @@ impl Date {
     /// let d = Date::today(0).unwrap();
     /// println!("The date today is: {}", d)
     /// ```
-    pub fn today(offset: i32) -> Result<Self, ParseError> {
-        Ok(DateTime::now(offset)?.date)
+    pub fn today(tz_offset: i32) -> Result<Self, ParseError> {
+        Ok(DateTime::now(tz_offset)?.date)
     }
 
     /// Day of the year, starting from 1.

diff --git a/src/datetime.rs b/src/datetime.rs
@@ -1,11 +1,11 @@
-use crate::{get_digit, Date, ParseError, Time};
+use crate::{Date, ParseError, Time};
 use std::cmp::Ordering;
 use std::fmt;
 use std::time::SystemTime;
 
 /// A DateTime
 ///
-/// Combines a [Date], [Time] and optionally a timezone offset in minutes.
+/// Combines a [Date], [Time].
 /// Allowed values:
 /// * `YYYY-MM-DDTHH:MM:SS` - all the above time formats are allowed for the time part
 /// * `YYYY-MM-DD HH:MM:SS` - `T`, `t`, ` ` and `_` are allowed as separators
@@ -25,51 +25,13 @@ pub struct DateTime {
     pub date: Date,
     /// time part of the datetime
     pub time: Time,
-    /// timezone offset in seconds if provided, must be >-24h and <24h
-    // This range is to match python,
-    // Note: [Stack Overflow suggests](https://stackoverflow.com/a/8131056/949890) larger offsets can happen
-    pub offset: Option<i32>,
 }
 
 impl fmt::Display for DateTime {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if self.time.microsecond != 0 {
-            let mut buf: [u8; 26] = *b"0000-00-00T00:00:00.000000";
-            crate::display_num_buf(4, 0, self.date.year as u32, &mut buf);
-            crate::display_num_buf(2, 5, self.date.month as u32, &mut buf);
-            crate::display_num_buf(2, 8, self.date.day as u32, &mut buf);
-            crate::display_num_buf(2, 11, self.time.hour as u32, &mut buf);
-            crate::display_num_buf(2, 14, self.time.minute as u32, &mut buf);
-            crate::display_num_buf(2, 17, self.time.second as u32, &mut buf);
-            crate::display_num_buf(6, 20, self.time.microsecond, &mut buf);
-            f.write_str(std::str::from_utf8(&buf[..]).unwrap().trim_end_matches('0'))?;
-        } else {
-            let mut buf: [u8; 19] = *b"0000-00-00T00:00:00";
-            crate::display_num_buf(4, 0, self.date.year as u32, &mut buf);
-            crate::display_num_buf(2, 5, self.date.month as u32, &mut buf);
-            crate::display_num_buf(2, 8, self.date.day as u32, &mut buf);
-            crate::display_num_buf(2, 11, self.time.hour as u32, &mut buf);
-            crate::display_num_buf(2, 14, self.time.minute as u32, &mut buf);
-            crate::display_num_buf(2, 17, self.time.second as u32, &mut buf);
-            f.write_str(std::str::from_utf8(&buf[..]).unwrap())?;
-        }
-        if let Some(offset) = self.offset {
-            if offset == 0 {
-                write!(f, "Z")?;
-            } else {
-                let mins = offset / 60;
-                let mut min = mins / 60;
-                let sec = (mins % 60).abs();
-                let mut buf: [u8; 6] = *b"+00:00";
-                if min < 0 {
-                    buf[0] = b'-';
-                    min = min.abs();
-                }
-                crate::display_num_buf(2, 1, min as u32, &mut buf);
-                crate::display_num_buf(2, 4, sec as u32, &mut buf);
-                f.write_str(std::str::from_utf8(&buf[..]).unwrap())?;
-            }
-        }
+        write!(f, "{}", self.date)?;
+        write!(f, "T")?;
+        write!(f, "{}", self.time)?;
         Ok(())
     }
 }
@@ -95,7 +57,7 @@ impl PartialOrd for DateTime {
     /// When comparing two datetimes, we want "less than" or "greater than" refer to "earlier" or "later"
     /// in the absolute course of time. We therefore need to be careful when comparing datetimes with different
     /// timezones. (If it wasn't for timezones, we could omit all this extra logic and thinking and just compare
-    /// struct members directly as we do with [Time], [Date] and [crate::Duration]).
+    /// struct members directly as we do with [Date] and [crate::Duration]).
     ///
     /// From [wikipedia](https://en.wikipedia.org/wiki/UTC_offset#Time_zones_and_time_offsets)
     ///
@@ -143,7 +105,7 @@ impl PartialOrd for DateTime {
     /// assert!(dt_france_4pm > dt_naive_330pm);
     /// ```
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        match (self.offset, other.offset) {
+        match (self.time.tz_offset, other.time.tz_offset) {
             (Some(_), Some(_)) => match self.timestamp_tz().partial_cmp(&other.timestamp_tz()) {
                 Some(Ordering::Equal) => self.time.microsecond.partial_cmp(&other.time.microsecond),
                 otherwise => otherwise,
@@ -182,8 +144,8 @@ impl DateTime {
     ///             minute: 13,
     ///             second: 14,
     ///             microsecond: 0,
+    ///             tz_offset: Some(0),
     ///         },
-    ///         offset: Some(0),
     ///     }
     /// );
     /// assert_eq!(dt.to_string(), "2022-01-01T12:13:14Z");
@@ -209,8 +171,8 @@ impl DateTime {
     ///             minute: 13,
     ///             second: 14,
     ///             microsecond: 0,
+    ///             tz_offset: Some(-30600),
     ///         },
-    ///         offset: Some(-30600),
     ///     }
     /// );
     /// assert_eq!(dt.to_string(), "2000-02-29T12:13:14-08:30");
@@ -246,8 +208,8 @@ impl DateTime {
     ///             minute: 13,
     ///             second: 14,
     ///             microsecond: 0,
+    ///             tz_offset: Some(0),
     ///         },
-    ///         offset: Some(0),
     ///     }
     /// );
     /// assert_eq!(dt.to_string(), "2022-01-01T12:13:14Z");
@@ -263,71 +225,9 @@ impl DateTime {
         }
 
         // Next try to parse the time
-        let (time, time_length) = Time::parse_bytes_partial(bytes, 11)?;
-        let mut position = 11 + time_length;
-
-        // And finally, parse the offset
-        let mut offset: Option<i32> = None;
-
-        if let Some(next_char) = bytes.get(position).copied() {
-            position += 1;
-            if next_char == b'Z' || next_char == b'z' {
-                offset = Some(0);
-            } else {
-                let sign = match next_char {
-                    b'+' => 1,
-                    b'-' => -1,
-                    226 => {
-                        // U+2212 MINUS "−" is allowed under ISO 8601 for negative timezones
-                        // > python -c 'print([c for c in "−".encode()])'
-                        // its raw byte values are [226, 136, 146]
-                        if bytes.get(position).copied() != Some(136) {
-                            return Err(ParseError::InvalidCharTzSign);
-                        }
-                        if bytes.get(position + 1).copied() != Some(146) {
-                            return Err(ParseError::InvalidCharTzSign);
-                        }
-                        position += 2;
-                        -1
-                    }
-                    _ => return Err(ParseError::InvalidCharTzSign),
-                };
-
-                let h1 = get_digit!(bytes, position, InvalidCharTzHour) as i32;
-                let h2 = get_digit!(bytes, position + 1, InvalidCharTzHour) as i32;
+        let time = Time::parse_bytes_offset(bytes, 11)?;
 
-                let m1 = match bytes.get(position + 2) {
-                    Some(b':') => {
-                        position += 3;
-                        get_digit!(bytes, position, InvalidCharTzMinute) as i32
-                    }
-                    Some(c) if c.is_ascii_digit() => {
-                        position += 2;
-                        (c - b'0') as i32
-                    }
-                    _ => return Err(ParseError::InvalidCharTzMinute),
-                };
-                let m2 = get_digit!(bytes, position + 1, InvalidCharTzMinute) as i32;
-
-                let minute_seconds = m1 * 600 + m2 * 60;
-                if minute_seconds >= 3600 {
-                    return Err(ParseError::OutOfRangeTzMinute);
-                }
-
-                let offset_val = sign * (h1 * 36000 + h2 * 3600 + minute_seconds);
-                // TZ must be less than 24 hours to match python
-                if offset_val.abs() >= 24 * 3600 {
-                    return Err(ParseError::OutOfRangeTz);
-                }
-                offset = Some(offset_val);
-                position += 2;
-            }
-        }
-        if bytes.len() > position {
-            return Err(ParseError::ExtraCharacters);
-        }
-
-        Ok(Self { date, time, offset })
+        Ok(Self { date, time })
     }
 
     /// Create a datetime from a Unix Timestamp in seconds or milliseconds
@@ -381,7 +281,6 @@ impl DateTime {
         Ok(Self {
             date,
             time: Time::from_timestamp(time_second, total_microsecond)?,
-            offset: None,
         })
     }
 
@@ -390,7 +289,7 @@ impl DateTime {
     ///
     /// # Arguments
     ///
-    /// * `offset` - timezone offset in seconds, must be less than `86_400`
+    /// * `tz_offset` - timezone offset in seconds, must be less than `86_400`
     ///
     /// # Examples
     ///
@@ -400,16 +299,16 @@ impl DateTime {
     /// let now = DateTime::now(0).unwrap();
     /// println!("Current date and time: {}", now);
     /// ```
-    pub fn now(offset: i32) -> Result<Self, ParseError> {
+    pub fn now(tz_offset: i32) -> Result<Self, ParseError> {
         let t = SystemTime::now()
             .duration_since(SystemTime::UNIX_EPOCH)
             .map_err(|_| ParseError::SystemTimeError)?;
         let mut now = Self::from_timestamp(t.as_secs() as i64, t.subsec_micros())?;
-        now.offset = Some(0);
-        if offset == 0 {
+        now.time.tz_offset = Some(0);
+        if tz_offset == 0 {
             Ok(now)
         } else {
-            now.in_timezone(offset)
+            now.in_timezone(tz_offset)
         }
     }
 
@@ -420,9 +319,9 @@ impl DateTime {
     ///
     /// # Arguments
     ///
-    /// * `offset` - optional timezone offset in seconds, set to `None` to create a naïve datetime.
+    /// * `tz_offset` - optional timezone offset in seconds, set to `None` to create a naïve datetime.
     ///
-    /// This method will return `Err(ParseError::OutOfRangeTz)` if `abs(offset)` is not less than 24 hours `86_400`.
+    /// This method will return `Err(ParseError::OutOfRangeTz)` if `abs(tz_offset)` is not less than 24 hours `86_400`.
     ///
     /// # Examples
     ///
@@ -434,16 +333,10 @@ impl DateTime {
     /// let dt2 = dt.with_timezone_offset(Some(-8 * 3600)).unwrap();
     /// assert_eq!(dt2.to_string(), "2022-01-01T12:13:14-08:00");
     /// ```
-    pub fn with_timezone_offset(&self, offset: Option<i32>) -> Result<Self, ParseError> {
-        if let Some(offset_val) = offset {
-            if offset_val.abs() >= 24 * 3600 {
-                return Err(ParseError::OutOfRangeTz);
-            }
-        }
+    pub fn with_timezone_offset(&self, tz_offset: Option<i32>) -> Result<Self, ParseError> {
         Ok(Self {
             date: self.date.clone(),
-            time: self.time.clone(),
-            offset,
+            time: self.time.with_timezone_offset(tz_offset)?,
         })
     }
 
@@ -454,7 +347,7 @@ impl DateTime {
     ///
     /// # Arguments
     ///
-    /// * `offset` - new timezone offset in seconds.
+    /// * `tz_offset` - new timezone offset in seconds.
     ///
     /// # Examples
     ///
@@ -466,13 +359,13 @@ impl DateTime {
     /// let dt_utc_plus2 = dt_z.in_timezone(7200).unwrap();
     /// assert_eq!(dt_utc_plus2.to_string(), "2000-01-01T17:00:00+02:00");
     /// ```
-    pub fn in_timezone(&self, offset: i32) -> Result<Self, ParseError> {
-        if offset.abs() >= 24 * 3600 {
+    pub fn in_timezone(&self, tz_offset: i32) -> Result<Self, ParseError> {
+        if tz_offset.abs() >= 24 * 3600 {
             Err(ParseError::OutOfRangeTz)
-        } else if let Some(current_offset) = self.offset {
-            let new_ts = self.timestamp() + (offset - current_offset) as i64;
+        } else if let Some(current_offset) = self.time.tz_offset {
+            let new_ts = self.timestamp() + (tz_offset - current_offset) as i64;
             let mut new_dt = Self::from_timestamp(new_ts, self.time.microsecond)?;
-            new_dt.offset = Some(offset);
+            new_dt.time.tz_offset = Some(tz_offset);
             Ok(new_dt)
         } else {
             Err(ParseError::TzRequired)
@@ -501,8 +394,8 @@ impl DateTime {
     /// Unix timestamp assuming epoch is in zulu timezone (1970-01-01T00:00:00Z) and accounting for
     /// timezone offset.
     ///
-    /// This is effectively [Self::timestamp] minus [Self::offset], see [Self::partial_cmp] for details on
-    /// why timezone offset is subtracted. If [Self::offset] if `None`, this is the same as [Self::timestamp].
+    /// This is effectively [Self::timestamp] minus [Self.time::tz_offset], see [Self::partial_cmp] for details on
+    /// why timezone offset is subtracted. If [Self.time::tz_offset] if `None`, this is the same as [Self::timestamp].
     ///
     /// # Examples
     ///
@@ -519,8 +412,8 @@ impl DateTime {
     /// assert_eq!(dt_plus_1.timestamp_tz(), 23 * 3600);
     /// ```
     pub fn timestamp_tz(&self) -> i64 {
-        match self.offset {
-            Some(offset) => self.timestamp() - (offset as i64),
+        match self.time.tz_offset {
+            Some(tz_offset) => self.timestamp() - (tz_offset as i64),
             None => self.timestamp(),
         }
     }