Looking for a review/input on adding Jiff support to rust_xlsxwriter

[jmcnamara]

Introduction

I wrote and maintain rust_xlsxwriter which is a Rust crate for creating new Excel xlsx files.

Currently the rust_xlsxwriter library supports a native ExcelDateTime type and also Chrono (as an optional crate feature). I would also like to add support for Jiff and have written a prototype implementation: jiff_to_excel with tests for a range of Excel dates and times.

Background

Datetimes in Excel are serial dates with days counted from an epoch (usually 1900-01-01) and where the time is a percentage/decimal of the milliseconds in the day. Both the date and time are stored in the same f64 value. For example, "2023/01/01 12:00:00" is stored as 44927.5.

Excel doesn't use timezones or try to convert or encode timezone information in any way.

Excel can also use a 1904-01-01 epoch to avoid the issues with the infamous 1900 Leap year bug. The 1904 epoch isn't widely used since Excel for Mac moved to the 1900 epoch and as such it isn't supported by rust_xlsxwriter although some of the structural code to support it is in place and the other language variants of xlsxwriter support it.

Excel can also save dates in a text ISO 8601 format when the file is saved using the "Strict Open XML Spreadsheet" option in the "Save" dialog. However this is rarely used in practice and isn't supported by rust_xlsxwriter.

Implementation

The prototype implementation is more or less as follows:

/// Convert a Jiff civil `DateTime` to an Excel serial datetime.
pub fn jiff_datetime_to_excel(datetime: &jiff::civil::DateTime) -> f64 {
    let date = jiff_date_to_excel(&datetime.date());
    let time = jiff_time_to_excel(&datetime.time());

    date + time
}

/// Convert a Jiff civil `Date` to an Excel serial datetime.
pub fn jiff_date_to_excel(date: &jiff::civil::Date) -> f64 {
    let epoch = jiff::civil::date(1899, 12, 31);
    let duration = *date - epoch;

    let mut excel_date = f64::from(duration.get_days());

    // Excel treats 1900 as a leap year so we need to add an additional day for
    // dates after the leapday.
    if excel_date > 59.0 {
        excel_date += 1.0;
    }

    excel_date
}

/// Convert a Jiff civil `Time` to an Excel serial datetime.
pub fn jiff_time_to_excel(time: &jiff::civil::Time) -> f64 {
    let midnight = jiff::civil::time(0, 0, 0, 0);
    let duration = *time - midnight;

    duration.total(jiff::Unit::Millisecond).unwrap() / (24.0 * 60.0 * 60.0 * 1000.0)
}

The full implementation is here.

The conversions would be opaque to the end user and would be behind the IntoExcelDateTime trait so that jiff::civil::DateTime|Date|Time instances could be written by rust_xlsxwriter like any other supported Rust or Excel data type.

The request

I would be grateful if you could review the implementation and make any suggestions.

In particular I'd like to know if it is better to use SignedDuration instead of Span (as used above).

Also, since Excel can't use the timezone information what is a suitable Cargo.toml include for jiff to get the minimal feature set to support only the "civil" functions. In relation to this I acknowledge that using Jiff without the timezone support omits a lot of the utility of the library.

[BurntSushi]

That all looks right to me. I think using a Span here is more convenient. It's possible using SignedDuration will be faster. But you'd have to benchmark it. It doesn't matter too much here because:

1. You're only dealing with civil datetimes.
2. You're only dealing with units of days or lower, which for civil datetimes, are invariant in length.

Otherwise, the only other thing I'd comment on is that I don't see any conversions from Excel datetimes to Jiff types. That direction will have a little more trickery to it since I presume it will be fallible.

Also, since Excel can't use the timezone information what is a suitable Cargo.toml include for jiff to get the minimal feature set to support only the "civil" functions. In relation to this I acknowledge that using Jiff without the timezone support omits a lot of the utility of the library.

You should be able to just disable all features. You shouldn't even need alloc. It's possible that in Jiff 1.0, you might need to enable a feature depending on the final configuration that I settle on. See #372 for more on that. But as it stands, most of Jiff's features are related to time zones.

[jmcnamara]

Thanks for the comments.

the only other thing I'd comment on is that I don't see any conversions from Excel datetimes to Jiff types. That direction will have a little more trickery to it since I presume it will be fallible.

The conversion is unidirectional in this case. rust_xlsxwriter only writes to Excel, it doesn't read data or round-trip it.

[jmcnamara]

Just to follow up on this. I added Jiff support to rust_xlsxwriter v0.88.0 under the crate feature flag jiff. With that enabled you can write instances of the Jiff civil types to Excel like the other supported type. Something like this:

use rust_xlsxwriter::{Format, Workbook, XlsxError};

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

    // Add a worksheet to the workbook.
    let worksheet = workbook.add_worksheet();

    // Create some formats to use with the datetimes below.
    let format1 = Format::new().set_num_format("dd/mm/yyyy hh:mm");
    let format2 = Format::new().set_num_format("mm/dd/yyyy hh:mm");
    let format3 = Format::new().set_num_format("yyyy-mm-ddThh:mm:ss");
    let format4 = Format::new().set_num_format("ddd dd mmm yyyy hh:mm");
    let format5 = Format::new().set_num_format("dddd, mmmm dd, yyyy hh:mm");

    // Set the column width for clarity.
    worksheet.set_column_width(0, 30)?;

    // Create a datetime object.
    let datetime = jiff::civil::datetime(2023, 1, 25, 12, 30, 0, 0);

    // Write the datetime with different Excel formats.
    worksheet.write_datetime_with_format(0, 0, datetime, &format1)?;
    worksheet.write_datetime_with_format(1, 0, datetime, &format2)?;
    worksheet.write_datetime_with_format(2, 0, datetime, &format3)?;
    worksheet.write_datetime_with_format(3, 0, datetime, &format4)?;
    worksheet.write_datetime_with_format(4, 0, datetime, &format5)?;

    workbook.save("worksheet.xlsx")?;

    Ok(())
}

Which gives the following output:

[BurntSushi]

Nice work! Thanks for working on it and thanks for asking for feedback. :-)