jmcnamara / rust_xlsxwriter

A Rust library for creating Excel XLSX files.
https://crates.io/crates/rust_xlsxwriter
Apache License 2.0
250 stars 23 forks source link

Bug: `value_format` annotation not working with `chrono::NaiveDate` #75

Closed lucatrv closed 6 months ago

lucatrv commented 6 months ago

Current behavior

Applying a value format annotation such as #[xlsx(value_format = "dd/mm/yyyy")] to a chrono::NaiveDate type field does not work, the field is serialized in the standard "yyyy-mm-dd" format. The same annotation works if a rust_xlsxwriter::ExcelDateTime type is used instead.

Expected behavior

The chrono::NaiveDate type filed should be serialized as "dd/mm/yyyy".

Sample code to reproduce

use chrono::NaiveDate;
use rust_xlsxwriter::{Workbook, XlsxError, XlsxSerialize};
use serde::Serialize;

fn main() -> Result<(), XlsxError> {
    let mut workbook = Workbook::new();
    let worksheet = workbook.add_worksheet();

    #[derive(Serialize, XlsxSerialize)]
    #[serde(rename_all = "PascalCase")]
    #[xlsx(header_format = 
        Format::new()
            .set_bold()
            .set_border(FormatBorder::Thin)
            .set_background_color("C6E0B4"))]
    struct Student<'a> {
        name: &'a str,
        age: u8,
        #[xlsx(value_format = "dd/mm/yyyy", column_width = 12.0)]
        birthdate: NaiveDate,
    }

    let students = [
        Student {
            name: "Aoife",
            age: 1,
            birthdate: NaiveDate::from_ymd_opt(2022, 11, 12).unwrap(),
        },
        Student {
            name: "Caoimhe",
            age: 2,
            birthdate: NaiveDate::from_ymd_opt(2022, 1, 2).unwrap(),
        },
    ];

    worksheet.set_serialize_headers::<Student>(1, 3)?;
    worksheet.serialize(&students)?;
    workbook.save("serialize.xlsx")?;

    Ok(())
}

Environment

- `rust_xlsxwriter` version: 0.61.0

Any other information

No response

jmcnamara commented 6 months ago

Thanks for the detailed report.

The reason for this is explained in the docs on Serializing dates and times:

The ExcelDateTime type is serialized automatically since it implements the Serialize trait. The Chrono types also implements Serialize but they will serialize to an Excel string in RFC3339 format. To serialize them to an Excel number/datetime format requires a serializing function like Utility::serialize_chrono_naive_to_excel() (as shown in the example below) or Utility::serialize_chrono_option_naive_to_excel().

Since the default chrono serialisation give a string the number format isn't applied (by Excel). Also note the dates are aligned to the left which indicates (using Excel's convention) that they are strings.

Following the guidance above and changing your sample app to suit:

use chrono::NaiveDate;
use rust_xlsxwriter::{Workbook, XlsxError, XlsxSerialize};
use rust_xlsxwriter::utility::serialize_chrono_naive_to_excel; // NEW.
use serde::Serialize;

fn main() -> Result<(), XlsxError> {
    let mut workbook = Workbook::new();
    let worksheet = workbook.add_worksheet();

    #[derive(Serialize, XlsxSerialize)]
    #[serde(rename_all = "PascalCase")]
    #[xlsx(header_format =
        Format::new()
            .set_bold()
            .set_border(FormatBorder::Thin)
            .set_background_color("C6E0B4"))]
    struct Student<'a> {
        name: &'a str,
        age: u8,
        #[xlsx(value_format = "dd/mm/yyyy", column_width = 12.0)]
        #[serde(serialize_with = "serialize_chrono_naive_to_excel")] // NEW.
        birthdate: NaiveDate,
    }

    let students = [
        Student {
            name: "Aoife",
            age: 1,
            birthdate: NaiveDate::from_ymd_opt(2022, 11, 12).unwrap(),
        },
        Student {
            name: "Caoimhe",
            age: 2,
            birthdate: NaiveDate::from_ymd_opt(2022, 1, 2).unwrap(),
        },
    ];

    worksheet.set_serialize_headers::<Student>(1, 3)?;
    worksheet.serialize(&students)?;
    workbook.save("serialize.xlsx")?;

    Ok(())
}

Output:

screenshot

Note that the dates are now aligned to the right which indicates (using Excel's convention) that they are numbers/dates.

lucatrv commented 6 months ago

Sorry I missed that, thanks for the clear explanation.

lucatrv commented 6 months ago

@jmcnamara, my actual use case is a little bit more complex because I need to serialize Result<chrono::NaiveDate, String>, should Utility::serialize_chrono_naive_to_excel() and Utility::serialize_chrono_option_naive_to_excel() be extended to support also those cases?

jmcnamara commented 6 months ago

my actual use case is a little bit more complex because I need to serialize Result<chrono::NaiveDate, String>,

You could use your own handler. Something like this:

pub fn serialize_chrono_result<S>(
    value: &Result<impl IntoExcelDateTime, String>,
    serializer: S,
) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    match value {
        Ok(datetime) => serializer.serialize_f64(datetime.to_excel_serial_date()),
        Err(string) => serializer.serialize_str(string),
    }
}
lucatrv commented 6 months ago

Yes sure, my comment was to evaluate if the serialize_chrono*_naive_to_excel functions had to be improved, if you think it is not feasible or needed then I will proceed with a custom serialization function.

jmcnamara commented 6 months ago

if you think it is not feasible or needed then I will proceed with a custom serialization function.

I think it is probably not needed. Unless at some point a lot of people turn up here with the same issue. So for now I will close as an issue that is best handled by the end user.

lucatrv commented 6 months ago

@jmcnamara , would you accept a PR to add a serialize_chrono_result_naive_to_excel helper function? When reading Excel files with calamine, I usually deserialize to Result<chrono:NaiveDateTime, String> instead of Option<chrono:NaiveDateTime> in order to get also invalid values. Then I need to implement serialize_chrono_result_naive_to_excel manually in my code. I think this is a helpful function which would pair very well with the existing serialize_chrono_naive_to_excel and serialize_chrono_option_naive_to_excel functions. Thanks

jmcnamara commented 6 months ago

would you accept a PR to add a serialize_chrono_result_naive_to_excel helper function?

No. I still think this is too specific a use case to require a solution to be provided by rust_xlsxwriter. As I said above, if that changes and more people report this as a use case then I will add it.

In the meantime I suggest handling it with a helper function:

use chrono::NaiveDate;
use rust_xlsxwriter::{IntoExcelDateTime, Workbook, XlsxError, XlsxSerialize};
use serde::{Serialize, Serializer};

fn main() -> Result<(), XlsxError> {
    let mut workbook = Workbook::new();
    let worksheet = workbook.add_worksheet();

    #[derive(Serialize, XlsxSerialize)]
    struct Student<'a> {
        name: &'a str,

        #[serde(serialize_with = "serialize_chrono_result")]
        #[xlsx(value_format = "dd mmm yyyy", column_width = 12.0)]
        date: Result<NaiveDate, String>,
    }

    let students = [
        Student {
            name: "Aoife",
            date: Ok(NaiveDate::from_ymd_opt(2022, 11, 12).unwrap()),
        },
        Student {
            name: "Caoimhe",
            date: Err("Not found".to_string()),
        },
    ];

    worksheet.set_serialize_headers::<Student>(0, 0)?;
    worksheet.serialize(&students)?;
    workbook.save("serialize.xlsx")?;

    Ok(())
}

pub fn serialize_chrono_result<S>(
    value: &Result<impl IntoExcelDateTime, String>,
    serializer: S,
) -> Result<S::Ok, S::Error>
where
    S: Serializer,
{
    match value {
        Ok(datetime) => serializer.serialize_f64(datetime.to_excel_serial_date()),
        Err(string) => serializer.serialize_str(string),
    }
}

Output:

screenshot

lucatrv commented 6 months ago

For reference, I report here how I implement serialize_chrono_result_naive_to_excel, which is general for Result<impl IntoExcelDateTime, impl Serialize>:

fn serialize_chrono_result_naive_to_excel<S, T>(
    datetime: &Result<impl IntoExcelDateTime, T>,
    serializer: S,
) -> Result<S::Ok, S::Error>
where
    S: Serializer,
    T: Serialize,
{
    match datetime {
        Ok(datetime) => serializer.serialize_f64(datetime.to_excel_serial_date()),
        Err(data) => T::serialize(data, serializer),
    }
}