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

Invalid decimal format in xl\charts\chart1.xml working with Italian Culture #930

Closed
MarcoBettio opened this issue Sep 29, 2022 · 7 comments

Comments

@MarcoBettio
Copy link

Generating a workbook with data a series containing decimal values and a line chart and saving it to xlsx, under the Italian CultureInfo, NPOI save in xl\charts\Chart1.xml all double values with comma instead of dot as decimal separator.
Opening the generated file with excel give an error and the option to recover the workbook, then it will fix the number format and the file finally opens.

I tried exporting to xlsx the same file but setting in my application the CurrentCulture to InvariantCulture and NPOI write the file in the correct way.

@PBrunot
Copy link
Contributor

PBrunot commented Oct 9, 2022

Could you please provide reproduction code for investigation?

@MarcoBettio
Copy link
Author

Just use the example code on your repository:
https://github.com/nissl-lab/npoi-examples/tree/main/xssf/LineChart

and change line 60 of Program.cs from this:
cell.SetCellValue(colIndex * (rowIndex + 1));
to this to have decimal values:
cell.SetCellValue((colIndex * (rowIndex + 1)) + 0.4);

In my case executing it generate an xlsx file that is corrupted (Italian Culture has comma for decimal separator).

Adding
using System.Globalization;
and at the beginning of the code
CultureInfo.DefaultThreadCurrentCulture = CultureInfo.InvariantCulture;

Make it just work as expected.

@PBrunot
Copy link
Contributor

PBrunot commented Oct 12, 2022

I suspect the issue is in this section of the generate document with Italian culture has XML code:
image

Same file generated with Invariant Culture has this XML code:
image

I am opening the files with Italian Excel version and reproduce the issue.

analysis

The different XML values are serialized CTNumVal.v properties, and this string is a double casted to string without culture, and then put in the XML file as it is.

Code filling the .v property:

*XSSFChartUtil:

 private static void FillNumCache<T>(CT_NumData cache, IChartDataSource<T> dataSource)
        {
            int numOfPoints = dataSource.PointCount;
            cache.AddNewPtCount().val = (uint)(numOfPoints);
            for (int i = 0; i < numOfPoints; ++i)
            {
                double value = Convert.ToDouble(dataSource.GetPointAt(i));
                if (!double.IsNaN(value))
                {
                    CT_NumVal ctNumVal = cache.AddNewPt();
                    ctNumVal.idx = (uint)(i);
                    ctNumVal.v = (value.ToString());
                }
            }
        }

XML fragment generation code:

*Chart.cs

  internal void Write(StreamWriter sw, string nodeName)
        {
            sw.Write(string.Format("<c:{0}", nodeName));
            XmlHelper.WriteAttribute(sw, "idx", this.idx, true);
            XmlHelper.WriteAttribute(sw, "formatCode", this.formatCode);
            sw.Write(">");
            if (this.v != null)
                sw.Write(string.Format("<c:v>{0}</c:v>", this.v));
            sw.Write(string.Format("</c:{0}>", nodeName));
        }

fixes?

One option can be to specify InvariantCulture in value.ToString() call, so that it gets serialized.
Or to reparse the string at the moment of serialization into a double, and serialize it as a string with invariant culture specified.
This second option is less clean but may be more compatible with code assuming CTNumVal.v is always current-culture formatted.

@tonyqus : any advice on your side to prepare the patch ?

Concept:

*Chart.cs

internal void Write(StreamWriter sw, string nodeName)
        {
            sw.Write(string.Format("<c:{0}", nodeName));
            XmlHelper.WriteAttribute(sw, "idx", this.idx, true);
            XmlHelper.WriteAttribute(sw, "formatCode", this.formatCode);
            sw.Write(">");
            if (this.v != null)
            {
                // v value has been formatted as string as per current culture, but XML requires number to be formatted as invariant culture
                // so we need to convert it back to number and then to string
                double dblValue = 0.0;
                if (double.TryParse(this.v, out dblValue))
                {
                    sw.Write(string.Format("<c:v>{0}</c:v>", dblValue.ToString(CultureInfo.InvariantCulture));
                }
                else
                {
                    // Fall back to string
                    sw.Write(string.Format("<c:v>{0}</c:v>", this.v));
                }
                
            }
                
            sw.Write(string.Format("</c:{0}>", nodeName));
        }```



Test files attached:
[test-italian.xlsx](https://github.com/nissl-lab/npoi/files/9768626/test-italian.xlsx)
[test-invariant.xlsx](https://github.com/nissl-lab/npoi/files/9768627/test-invariant.xlsx)

@Bykiev
Copy link
Collaborator

Bykiev commented Oct 13, 2022

I had the same issue with charts and decimal separator, but it was closed

@tonyqus
Copy link
Member

tonyqus commented Oct 16, 2022

Option one (to specify InvariantCulture in value.ToString() call) looks safe. Please go ahead and create a PR.

@tonyqus tonyqus added this to the NPOI 2.6.1 milestone Oct 16, 2022
PBrunot added a commit to PBrunot/npoi that referenced this issue Oct 20, 2022
fix for nissl-lab#930 and nissl-lab#376 : force invariant culture as non-english version of Excel expect "1.23" format for 1.23 double and the double->string conversion is dependent on current culture, resulting in "1,23" being generated in the XML.
PBrunot added a commit to PBrunot/npoi that referenced this issue Oct 20, 2022
fix for nissl-lab#930 and nissl-lab#376 : force invariant culture as non-english version of Excel expect "1.23" format for 1.23 double and the double->string conversion is dependent on current culture, resulting in "1,23" being generated in the XML.
@MarcoBettio
Copy link
Author

Hi all, I write again because I was following the thread...
From the analisys of @PBrunot and the response of @tonyqus I was sure that the patch should be applied in this piece of code:

*XSSFChartUtil:

 private static void FillNumCache<T>(CT_NumData cache, IChartDataSource<T> dataSource)
        {
            int numOfPoints = dataSource.PointCount;
            cache.AddNewPtCount().val = (uint)(numOfPoints);
            for (int i = 0; i < numOfPoints; ++i)
            {
                double value = Convert.ToDouble(dataSource.GetPointAt(i));
                if (!double.IsNaN(value))
                {
                    CT_NumVal ctNumVal = cache.AddNewPt();
                    ctNumVal.idx = (uint)(i);
  HERE----> ctNumVal.v = (value.ToString());
                }
            }
        }

where there is already an ToString(), but the patch created touch the other file where the value was not converted anymore (and at this point I don't know if the conversion with InvariantCulture makes some difference...

@PBrunot
Copy link
Contributor

PBrunot commented Oct 29, 2022

You're right, the bug is still here, I messed up.

I'm now writing a reproduction case starting from LineChart.cs. as follows:

foreach(var culture in CultureInfo.GetCultures(CultureTypes.NeutralCultures)) { CultureInfo.DefaultThreadCurrentCulture = culture; Console.WriteLine($"Testing with {culture.Name}..."); GenerateFile(); }
Then my test procedure is to open the generated Excel will with my italian-language Excel 365.
Problem is there in many, many cultures.

I'll prepare a fix to rollback the previous change which did not help.

PBrunot added a commit to PBrunot/npoi that referenced this issue Oct 29, 2022
Tested with 277 cultures successfully
PBrunot added a commit to PBrunot/npoi that referenced this issue Oct 29, 2022
tonyqus added a commit that referenced this issue Dec 29, 2022
@tonyqus tonyqus closed this as completed Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants