tafia / calamine

A pure Rust Excel/OpenDocument SpreadSheets file reader: rust on metal sheets
MIT License
1.6k stars 155 forks source link

fix regression on Range::get #353

Closed lukapeschke closed 9 months ago

lukapeschke commented 10 months ago

Hello @tafia , and thanks a lot for your work on calamine!

It seems like #342 introduced a bug: the column # should be matched against the width, and the row # against the height. Example of a reproduction case:

[src/utils/arrow.rs:10] data = Range {
    start: (
        0,
        0,
    ),
    end: (
        1,
        3,
    ),
    inner: [
        String(
            "date",
        ),
        String(
            "datestr",
        ),
        String(
            "time",
        ),
        String(
            "datetime",
        ),
        DateTimeIso(
            "2023-06-01",
        ),
        String(
            "2023-06-01T02:03:04+02:00",
        ),
        DurationIso(
            "PT01H02M03S",
        ),
        DateTimeIso(
            "2023-06-01T02:03:04",
        ),
    ],
}
[src/utils/arrow.rs:10] data.get((1, 0)) = Some(
    DateTimeIso(
        "2023-06-01",
    ),
)
[src/utils/arrow.rs:10] data.get((1, 1)) = Some(
    String(
        "2023-06-01T02:03:04+02:00",
    ),
)
[src/utils/arrow.rs:10] data.get((1, 2)) = None

In case you're interested, we're maintaining a python wrapper around calamine, as pandas and polars 's performance with excel files is disappointing. calamine has been a huge win for us :smile: We caught the bug in fastexcel's CI : https://github.com/ToucanToco/fastexcel/pull/127

tafia commented 9 months ago

Thanks! Awesome project, thanks! Fyi, I also saw that @dimastbk (who's contributing a lot recently) also has a https://github.com/dimastbk/python-calamine wrapper.

lukapeschke commented 9 months ago

Ah great, I'll have a look!