Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Millisecond timestamps parsed incorrectly #61

Closed
latk opened this issue Mar 26, 2024 · 1 comment · Fixed by #65
Closed

Millisecond timestamps parsed incorrectly #61

latk opened this issue Mar 26, 2024 · 1 comment · Fixed by #65

Comments

@latk
Copy link

latk commented Mar 26, 2024

The DateTime::parse_str(...) constructor supports various different formats, but millisecond-granularity timestamps are parsed incorrectly when they have a fractional part.

Testcase to reproduce:

use speedate::DateTime;

fn timestamp_ms(dt: DateTime) -> i64 {
    dt.timestamp() * 1000 + i64::from(dt.time.microsecond / 1000)
}

#[test]
fn from_fractional_float_str() {
    assert_eq!(
        timestamp_ms(DateTime::parse_str("1711445175471.865").unwrap()),
        1711445175471
    )
}

Failure:

---- from_fractional_float_str stdout ----
thread 'from_fractional_float_str' panicked at tests/datetime_timestamp_bytes.rs:9:5:
assertion `left == right` failed
  left: 1711445176335
 right: 1711445175471

Analysis: the difference between the two values is 864, which is close to the fractional part .865. It seems that the fractional part of a float input is always interpreted on the seconds scale, even if the integral part of the input is interpreted on the milliseconds scale.

This is not a bug in the above timestamp_ms() helper. The same problem can be observed when accessing this functionality via Pydantic:

import pydantic  # v2.6.4
from datetime import datetime

class Model(pydantic.BaseModel):
    x: datetime

Model(x="1711445175471.865").x.timestamp() * 1000
#=> 1711445176335.99

Originally I though this was a bug in the custom float parsing which seems to accumulate rounding errors because it skips the checks in f64::from_str(), but I now think the issue is actually in the usage of the DateTime::from_timestamp(ts, micros) constructor by the parse_bytes_with_config() constructor:

speedate/src/datetime.rs

Lines 344 to 346 in ecbe681

IntFloat::Float(float) => {
let micro = (float.fract() * 1_000_000_f64).round() as u32;
Self::from_timestamp_with_config(float.floor() as i64, micro, config)

I am not sure how to fix this. It is impossible to call DateTime::from_timestamp(ts, micros) with a non-zero value for micros without already knowing how the ts part will be interpreted.

@davidhewitt
Copy link
Contributor

Thanks - agreed in your analysis and added a possible fix in #65

davidhewitt added a commit that referenced this issue Jun 26, 2024
* add tests for #61

* fix millisecond fraction being handled with wrong scale

* also raise error if fraction too long

* additional test cases

* update comment

* fix doctest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants