Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Number serialization does not use neutral culture #903

Closed
Nils-Berghs opened this issue Mar 2, 2021 · 5 comments
Closed

Number serialization does not use neutral culture #903

Nils-Berghs opened this issue Mar 2, 2021 · 5 comments
Labels

Comments

@Nils-Berghs
Copy link

When creating an excel file numbers are serialized with the culture of the current thread. However internally excel uses the invariant/US-culture.

So if you add a value to cell A1 like this:

// a value
double value = 0.5;
//assume sheet, row, are created
row.AppendChild(new Cell { CellReference = "A1", CellValue = new CellValue(value), DataType = new EnumValue<CellValues>(CellValues.Number)});

Excel will accept the file if the code above is executed in a thread running on US culture. If running on a thread that uses ',' as a decimal seperator, excel will give the "We found a problem witn some content..." warning.
If you examine the value in the zip file it will be "0,5" (not 0.5 as is required).

I have not found a work around other than forcing the the thread to use US culture.

Note that the culture settings of excel do not matter, it is the generation of the xlsx that is wrong.

@tomjebo
Copy link
Collaborator

tomjebo commented Mar 2, 2021

This looks like a similar issue as #288 except that our ctor is doing the ToString() without doing it like this: ToString(CultureInfo.InvariantCulture)
Perhaps we should change the ctors to use the above method? @twsouthwick what do you think?

@tomjebo
Copy link
Collaborator

tomjebo commented Mar 2, 2021

@Nils-Berghs I've added PR#904 but don't have a non-US English machine handy. Can you take a look and test your scenario?

@Nils-Berghs
Copy link
Author

@tomjebo I have tested the branch you are merging and this seems ok.

@tomjebo
Copy link
Collaborator

tomjebo commented Mar 3, 2021

@Nils-Berghs thanks for testing. Doing one more confirmation internally and then I'll merge this.

@tomjebo
Copy link
Collaborator

tomjebo commented Mar 6, 2021

I've merged #904, added PR #906 for testing double and decimal CellValue.
Closing this issue.

@tomjebo tomjebo closed this as completed Mar 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants