nyx-space/hifitime

`TimeSeries` implements `core::iter::ExactSizeIterator` but doesn't provide upper bound on `size_hint`

d3v-null opened this issue · 4 comments

d3v-null commented

Hello!
Big fan of this library by the way, love your work.

I wanted to see if I could get the length of a TimeSeries without collecting into a Vec, but when I call .len() I get the error below.

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(0)`', /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/iter/traits/exact_size.rs:108:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:182:5
   4: core::iter::traits::exact_size::ExactSizeIterator::len
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/iter/traits/exact_size.rs:108:9
   5: hifitimetest::main
             at ./src/bin/hifitimetest.rs:9:10
   6: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

This seems to be because ExactSizeIterator is expecting an upper bound on size_hint, which I found to always be None.

If there's no intention of providing .len(), and TimeSeries impls ExactSizeIterator for some other reason, then that's understandable, but it's just not clear from the docco.

Here's some code to demo what I mean.

use hifitime::{Duration, Epoch, TimeSeries, Unit::Microsecond};

fn main() {
    let start = Epoch::from_gregorian_str("2022-07-14T02:56:11.228271007 UTC").unwrap();
    let step = Duration::from_f64(0.5, Microsecond);
    let steps = 100_000_000_000_000;
    let end = start + steps * step;
    let times = TimeSeries::exclusive(start, end, step);
    dbg!(times.size_hint());
    assert_eq!(times.len(), steps as usize);
}

my cargo.toml:

[package]
...
edition = "2021"
rust-version = "1.60"

[dependencies]
hifitime = "3.2.0"

I get

[src/bin/hifitimetest.rs:8] times.size_hint() = (
    0,
    None,
)

and then the error above.

Not a big deal since you can do .count(), it just consumes the iterator and takes a surprisingly long time.

fn main() {
    let start = Epoch::from_gregorian_str("2022-07-14T02:56:11.228271007 UTC").unwrap();
    let step = Duration::from_f64(0.5, Microsecond);
    let steps = 1_000_000_000;
    let end = start + 1_000_000_000 * step;
    let times = TimeSeries::exclusive(start, end, step);

    let timer_start = Epoch::now().unwrap();
    dbg!(times.size_hint());
    assert_eq!(times.count(), steps as usize);
    let timer_end = Epoch::now().unwrap();
    eprintln!("took {}seconds", (timer_end - timer_start).in_seconds());
}
[src/bin/hifitimetest.rs:11] times.size_hint() = (
    0,
    None,
)
took 65.855730688seconds

version info

➜ cargo -Vv
cargo 1.61.0 (a028ae4 2022-04-29)
release: 1.61.0
commit-hash: a028ae42fc1376571de836be702e840ca8e060c2
commit-date: 2022-04-29
host: x86_64-unknown-linux-gnu
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1m)
os: Arch Linux [64-bit]

➜ cargo tree --package hifitime
hifitime v3.2.0
├── libm v0.2.2
├── regex v1.6.0
│   ├── aho-corasick v0.7.18
│   │   └── memchr v2.5.0
│   ├── memchr v2.5.0
│   └── regex-syntax v0.6.27
├── serde v1.0.139
└── serde_derive v1.0.139 (proc-macro)
    ├── proc-macro2 v1.0.40
    │   └── unicode-ident v1.0.1
    ├── quote v1.0.20
    │   └── proc-macro2 v1.0.40 (*)
    └── syn v1.0.98
        ├── proc-macro2 v1.0.40 (*)
        ├── quote v1.0.20 (*)
        └── unicode-ident v1.0.1

Thanks!

ChristopherRabotin commented
d3v-null commented

No worries! enjoy your vacation!

ChristopherRabotin commented

Sorry for the delay here, I'm working on this now. I hope to get it fixed very soon and it's scheduled in next weekend's release: https://github.com/nyx-space/hifitime/milestone/8 .

Let me know if there are other iterator functions you expect to use so I can make sure that those aren't buggy like size_hint is.

  • 👍1
ChristopherRabotin commented

Sorry I only got to this now. I think I've fixed it, and used your test case in the tests themselves (although I changed the number of steps so I could do the math in my head): https://github.com/nyx-space/hifitime/pull/134/files#diff-1ddffea2426780693e1e8ab9f0cf7dcf9361f71126303ebef15d39d919e03cedR173 . I think this is correct but let me know if it isn't.

Thanks again for the bug report!