time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.13k stars 281 forks source link

Formatting bug on ISO8601 #678

Closed dracarys18 closed 6 months ago

dracarys18 commented 7 months ago

Hi @jhpratt continuing our discussion from #674. I have created a repo to reproduce this bug here. You can try to run it and check it yourself.

cargo r | tee test.txt
cat test.txt | rg ":60."

This looks like this happens only when the TimePrecision is 3 here.

    const FORMAT_CONFIG: EncodedConfig = Config::DEFAULT
        .set_time_precision(TimePrecision::Second {
            decimal_digits: NonZeroU8::new(3),
        })
        .encode();

It looks like it's rounding off the time to 60 seconds and it's happening every minute. If I change the precision to 6 I can see it's adjusting to nano second

jhpratt commented 7 months ago

Odd. I've confirmed that the code as-is does what you say, so I'll definitely look into this more.

jhpratt commented 7 months ago

It looks like it's rounding off the time to 60 seconds and it's happening every minute.

Confirmed. This appears to be the behavior of Display for floats, and to be fair it is what 99.9% of people would expect (as it's the closest to the true value). I naïvely assumed that it would truncate, and a quick search just now reveals that the behavior isn't actually documented.

Given that I can't rely on the standard library's implementations here, I'm going to have to manually format floats. It's doable and I'll probably get to it this weekend.

dracarys18 commented 7 months ago

It looks like it's rounding off the time to 60 seconds and it's happening every minute.

Confirmed. This appears to be the behavior of Display for floats, and to be fair it is what 99.9% of people would expect (as it's the closest to the true value). I naïvely assumed that it would truncate, and a quick search just now reveals that the behavior isn't actually documented.

Given that I can't rely on the standard library's implementations here, I'm going to have to manually format floats. It's doable and I'll probably get to it this weekend.

Sure @jhpratt, I can take this up if you just give me a place where I should start looking at.

jhpratt commented 7 months ago

The specific function is format_float in src/formatting/mod.rs. The Some arm is what is relevant.

dracarys18 commented 7 months ago

The specific function is format_float in src/formatting/mod.rs. The Some arm is what is relevant.

Sure I will take a look and create a PR for this. Thanks!

dracarys18 commented 7 months ago

Hey @jhpratt, So this can be fixed by just truncating the float number to the precision point before we pass it to formatter. Playground link for it. This would require just a one line change in format_float function where we would just truncate value before it we pass it in write!. Let me know if you have better suggestions, Otherwise I will create a PR for this.