MannLabs / timsrust

Apache License 2.0
20 stars 9 forks source link

[Question] Origin of the `scan_max_index` value #18

Closed jspaezp closed 1 week ago

jspaezp commented 2 weeks ago

Hello!

I was wondering that the origin of this hard-coded value is.

https://github.com/MannLabs/timsrust/blob/4001f8973b16abd0e5bbb066f408c720e99105ef/src/file_readers/common/sql_reader/metadata.rs#L64

It would be great to add it as a comment in the code.

Kindest wishes, Sebastian

sander-willems-bruker commented 1 week ago

That is an artefact of "getting it to work first before doing it proper" I am afraid. Worked on my test dataset and a lot of other files, but indeed is not impemented correctly and should be fixed in the upcoming 0.3.0 version that has some major refactoring to hopefully get a lot cleaner code. Thanks for catching it!

sander-willems-bruker commented 1 week ago

Fixed:

https://github.com/MannLabs/timsrust/blob/develop/src/io/readers/metadata_reader.rs#L66-L69

jspaezp commented 1 week ago

Thanks a lot!

for future me this is the change in implementation:

    // let scan_max_index: u32 = 927; 
    let scan_counts: Vec<u32> = tdf_sql_reader
        .read_column_from_table("NumScans", "Frames")
        .unwrap();
    let scan_max_index = *scan_counts.iter().max().unwrap();