search

unitsofmeasurement / uom-systems

Units of Measurement Systems
http://www.uom.systems
Other
36 stars 17 forks source link

UCUMFormat is not correctly appending superscript powers when formatting units #135

Closed mattdehring closed 4 years ago

mattdehring commented 5 years ago

If a unit like m3/s is input the output is m/s3 instead. It also appears that if there are multiple units in the denominator it doesn't correctly add parenthesis around them. For instance if you have an input of m/(s.bar) the output is m/s/b.

mattdehring commented 5 years ago

I made a PR that has unit tests for the problems and a solution for it

cleberecht commented 5 years ago

I just run into the same problem. Thanks for the fix.

alexanderkiel commented 5 years ago

Unfortunately you introduced a new bug with commit 2fdd93da6c. kg/m2 formats to kg/m now. The fix is simple, you just have an Math.abs too much. You can find it here:

https://github.com/unitsofmeasurement/uom-systems/blob/6ba1e8b0a0cba6764a5189d21fa4d550b3cb9b92/ucum-java8/src/main/java/systems/uom/ucum/format/UCUMFormat.java#L253

keilw commented 5 years ago

That does not seem new with 1.0 though which only incorporates SI 1.0, could you propose a line instead of if (Math.abs(u.getValue()) < -1){ that fixes the issue in your experience or propose a PR?

mattdehring commented 5 years ago

I am not sure about this process who should make the fix? It should just be changed to if ( u.getValue() < -1){. This needs to be done for both versions.

alexanderkiel commented 5 years ago

Thanks @keilw @mattdehring I decided agains a PR because I don't oversee all locations it has to be fixed.

keilw commented 5 years ago

So without the abs(), we'll try that. Because the SI redefinition affects the SI system modules and probably in some cases also UCUM directly, we expect one or the other update in the 1.x branch independent of the 2.x (master) branch.

alexanderkiel commented 5 years ago

Would you recommend to use the 2.x branch instead? I remember having some problems using the whole 2.x system. But not exactly how. I only need to parse and format unit and quantities for calculation.

keilw commented 5 years ago

2.0 of Unit-API is not final yet, but if you are fine with a SNAPSHOT release, it works and will be the one taking over soon.

alexanderkiel commented 5 years ago

Ok thanks. Now I understand. No, I can't use a SNAPSHOT.

keilw commented 5 years ago

Great, than please use 1.x at least till around late Q2, then all 2.x versions based on Indriya 2 Final and JSR 385 should be available. And if you used uom-se or the -java8 variants, those work on identical environments. The minimal JRE should be Java SE 8 in most cases, on occasion we might raise it to 9-11, especially if new features are used like JShell or the var keyword.

alexanderkiel commented 5 years ago

Thank you a lot. I use 1.x in -java8 variant with Java SE 8 but might be update to Java SE 11 later this year.

sirgrigio commented 5 years ago

Hello, is there any news about this issue? I am using version 2.0 (not snapshot) and 

UCUMFormat.getInstance(UCUMFormat.Variant.CASE_SENSITIVE).format(UCUM.GRAM.divide(UCUM.METER.pow(3)))

produces g/m instead of g/m3.

keilw commented 4 years ago

As demonstrated in UCUMFormatDemo of the UCUM console demos, this was fixed by #162 in 2.0.2, please test the SNAPSHOT, we plan to release it soon.