informationsea / xlsxwriter-rs

Excel file writer for Rust
https://crates.io/crates/xlsxwriter
Apache License 2.0
265 stars 45 forks source link

Add AutoMinimum ConditionalFormatRuleType enum #59

Closed tedbrakob closed 1 week ago

tedbrakob commented 1 week ago

Automatic is the default minimum type for data bars in Excel. It differs from the Minimum by defaulting to 0 if no negative values are in the range. The value will be used if there are negative values in the range.

The constant already exists at libxlsxwriter_sys::lxw_conditional_format_rule_types_LXW_CONDITIONAL_RULE_TYPE_AUTO_MIN, this PR adds it to the allowed enum of ConditionalFormatRuleTypes

tedbrakob commented 1 week ago

@informationsea, I'm planning on using this minimum type on a project I'm currently working on. I would really appreciate a quick merge/release if you have the time! 😀

jmcnamara commented 1 week ago

@tedbrakob

That LXW_CONDITIONAL_RULE_TYPE_AUTO_MIN enum value is, more or less, an internal value used for defaults. What is the use case for needing to set it? Does it not just get set automatically? Could you maybe show an example.

There is also an LXW_CONDITIONAL_RULE_TYPE_AUTO_MAX value. If MIN is needed then MAX should be added too. If it is needed.

The reason I ask is to see if this something missing from libxlsxwriter. That is why a code example of your use case would be useful.

tedbrakob commented 1 week ago

@jmcnamara

Thanks for looking over this. You're right that this won't work how I was expecting. I expected it to align with the Automatic option in the data bar options here: image

In my testing, this is still resulting in "Lowest Value" being set as the min type.

I can see in libxlsxwriter, there is an internal type lxw_cond_format_obj with a property of auto_min which looks like it controls the setting I'm looking for, but as far as I can tell, there's no way to change that value.

Any advice on how to use the "Automatic" min type, or is that not currently supported in libxlsxwriter?

jmcnamara commented 1 week ago

In my testing, this is still resulting in "Lowest Value" being set as the min type.

@tedbrakob You will need to add a working example in code. The library should always default to "Automatic" if no other type is set.

tedbrakob commented 1 week ago

It's possible that I was testing the LXW_CONDITIONAL_RULE_TYPE_AUTO_MIN rule improperly and getting a different result because of that. I don't have time to set up this library for local testing again, so I trust you're right about the C library.

As for the Rust libary's default values: the min rule type defaults to ConditionalFormatRuleTypes::Minimum with no way of changing it to Automatic since there isn't a rule type yet.

https://github.com/informationsea/xlsxwriter-rs/blob/b737fb0e0208c7f5b1dbee03209bcd1f35a450f0/libxlsxwriter/src/worksheet/conditional_format/data_bar.rs#L75

jmcnamara commented 1 week ago

I don't have time to set up this library for local testing again

A rust example with xlsxwriter.rs is fine.

tedbrakob commented 2 days ago

@jmcnamara Here's an example using the example listed at the bottom of this repo's readme. https://github.com/tedbrakob/xlsxwriter-rs/blob/databars/libxlsxwriter/examples/hello_spreadsheet.rs

The default data bar conditional formatting here results in the use of ConditionalFormatRuleTypes::Minimum as can be seen in the resulting file.

image

Using the Automatic rule would result in data bars that look like this, with 0 as the minimum of the data bar range: (ignoring the color change)

image

Using an AutoMin type mapped to lxw_conditional_format_rule_types_LXW_CONDITIONAL_RULE_TYPE_AUTO_MIN has no effect on the output

jmcnamara commented 2 days ago

@tedbrakob Thanks for the example.

That doesn't actually have anything to do with the minimum/automatic setting. Instead it is related to style of the databar conditional format.

Excel 2007 originally supported the type of databar shown in the first example. This was extended in Excel 2010 with some additional settings. One of these is that it automatically picks the minimum value instead of 0.

It is possible to set this in the C library:


#include "xlsxwriter.h"

int main()
{

    lxw_workbook *workbook = workbook_new("conditional_format.xlsx");
    lxw_worksheet *worksheet8 = workbook_add_worksheet(workbook, NULL);

    /* Write the worksheet data. */
    for (int i = 1; i <= 12; i++)
    {
        worksheet_write_number(worksheet8, i + 1, 1, i + 19, NULL);
        worksheet_write_number(worksheet8, i + 1, 3, i + 19, NULL);
    }

    lxw_conditional_format *conditional_format =
        (lxw_conditional_format *)calloc(1, sizeof(lxw_conditional_format));

    /* Write an original Excel 2007 style databar. */
    conditional_format->type = LXW_CONDITIONAL_DATA_BAR;
    worksheet_conditional_format_range(worksheet8, RANGE("B3:B14"), conditional_format);

    /* Write an Excel 2010 style databar. */
    conditional_format->type          = LXW_CONDITIONAL_DATA_BAR;
    conditional_format->data_bar_2010 = LXW_TRUE;
    worksheet_conditional_format_range(worksheet8, RANGE("D3:D14"), conditional_format);

    free(conditional_format);
    return workbook_close(workbook);
}

Which gives this output:

screenshot

It looks like xlsxwriter-rs doesn't have a method to set the data_bar_2010 parameter.

You could patch it to support that setting or you could use rust_xlsxwriter which uses Excel 2010 databar by default:

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

fn main() -> Result<(), XlsxError> {
    // Create a new Excel file object.
    let mut workbook = Workbook::new();
    let worksheet = workbook.add_worksheet();

    // Write the worksheet data.
    let data = [20, 21, 22, 23 ,24, 25, 26, 27, 28, 29, 30, 31];
    worksheet.write_column(2, 1, data)?;

    // Write a standard Excel data bar.
    let conditional_format = ConditionalFormatDataBar::new();

    worksheet.add_conditional_format(2, 1, 13, 1, &conditional_format)?;

    // Save the file.
    workbook.save("conditional_format.xlsx")?;

    Ok(())
}

Output:

screenshot

tedbrakob commented 1 day ago

@jmcnamara Thanks for all the info!