Unity Loader: fixed float parsing for different globalization styles

[JayKay135]

Countries have different national styles for the decimal separation of doubles/ floates.
CultureInfo.InvariantCulture makes sure to parse the String correctly based on the users device nationality.

[gkjohnson]

Hello! Thanks for the contribution. Can you point me to ROS or URDF spec documentation where it states that multiple number formats are considered valid?

[JayKay135]

I had a another thought on this subject. I come from Germany, where we (unfortunately) use the ',' as decimal separator. And it puzzled me that the URDF sample files provided did not parse properly. This is because the URDF files used the '.' as a separator. But one would assume that a foreigner with different language standards would then also use this with their URDF files. After that, the files should also all parse correctly again. So again sorry for my wrong assumption. But you can ignore my suggested changes.

[gkjohnson]

Sorry - I think I'm not understanding. Are you saying that the sample files provided do not parse correctly on your machine with the parser as-is?

[JayKay135]

Sorry for the late reply.
Yes exactly.

Here as an example result from my German machine:

double testA = double.Parse("1.234"); // returns 1234
double testB = double.Parse("1,234"); // returns 1,234

The provided sample urdf files will therefore only parse correctly in countries that use the '.' as a decimal separator.

[gkjohnson]

Yes exactly.
Here as an example result from my German machine:

Thanks! I understand, now. I didn't expect this is how things would work - it seems like it could only cause problems with code running differently in different countries as we're seeing... In this case I think it's worth passing a CultureInfo option that ensures that floating point values with periods are parsed on every machine. But I'd prefer if commas are not parsed as float separators unless there's a field in the URDF specification that specifies commas are acceptable.

I'm having a hard time finding the docs on the exact behavior of CultureInfo.InvariantCulture but it sounds like it would cause parsing of both commas and periods as float values, correct?

[JayKay135]

I see your point.
I've also tried to find anything regarding decimal separators in the official ROS documentation.
Due to the absense of any specification I assume that '.' and ',' are both valid separators according to the equivalent country usage.

Regarding your question about CultureInfo.InvariantCulture:

I've expanded my testing:

double testA = double.Parse("1.234"); // returns 1234
double testB = double.Parse("1,234"); // returns 1,234

double testC = double.Parse("1.234", CultureInfo.InvariantCulture); // returns 1,234
double testD = double.Parse("1,234", CultureInfo.InvariantCulture); // returns 1234

It unfortunately parses ',' floats incorrectly with CultureInfo.InvariantCulture.

[gkjohnson]

Due to the absense of any specification I assume that '.' and ',' are both valid separators according to the equivalent country usage.

I don't agree with this. Unless there is explicit or demonstrable support for commas as decimal separators I think it's best to only support periods. Python is a core facility of ROS and afaict it does not support the same kind of implicit locality conversion. I haven't seen any URDF files that use commas internally and I feel it's best to not make a format parser that enables ambiguity in apparent support and encourages creation of files that may not load everywhere.

I've expanded my testing:
...
It unfortunately parses ',' floats incorrectly with CultureInfo.InvariantCulture.

Okay - this is the desired behavior in opinion so I think using InvariantCulture is good at least for now.

Thanks again!