tafia / calamine

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

Feature Request: please add method `RangeDeserializerBuilder::with_headers_struct` #395

Closed lucatrv closed 4 months ago

lucatrv commented 5 months ago

Often we need to write something like:

#[derive(Deserialize, Serialize)]
struct Record {
    #[serde(rename = "Property")]
    house: &'static str,
    #[serde(rename = "Price")]
    value: f64,
}

let iter_results =
    calamine::RangeDeserializerBuilder::with_headers(&["Property", "Price"]).from_range(&range)?;

However, for structs which derive Deserialize, it is possible to get the Serde field names from the struct type, see for instance the serde_aux::serde_introspection and rust_xlsxwriter::deserialize_headers implementations.

It would be very convenient to add a RangeDeserializerBuilder::with_headers_struct (or other name) in order to write:

#[derive(Deserialize, Serialize)]
struct Record {
    #[serde(rename = "Property")]
    house: &'static str,
    #[serde(rename = "Price")]
    value: f64,
}

let iter_results =
    calamine::RangeDeserializerBuilder::with_headers_struct::<Record>().from_range(&range)?;

@tafia if you agree I can implement this and issue a PR.

tafia commented 5 months ago

Sure! Why not directly something like from_range_with_header<T>?

lucatrv commented 5 months ago

Why not directly something like from_range_with_header<T>?

I though it would be better to keep aligned with RangeDeserializerBuilder::with_headers and possible other future methods RangeDeserializerBuilder::with_columns and Range::rows_with_columns, as discussed with feature request #314. They all specify the elements to keep, while the range is specified elsewhere. However I'm also good with from_range_with_headers::<T>(&range) if you prefer it, but then maybe there could also be a from_range_with_headers_slice(&range, &headers).

Another option to consider would be to deprecate the current RangeDeserializerBuilder::with_headers, and keep only RangeDeserializerBuilder::from_range, RangeDeserializerBuilder::from_range_with_headers, and RangeDeserializerBuilder::from_range_with_columns.

Please let me know your preference.

lucatrv commented 4 months ago

After thinking a little bit more about this, IMHO the best choice is RangeDeserializerBuilder::with_deserialize_headers.