search

unitsofmeasurement / uom-se

JSR 363 - Implementation for Java SE 8
Other
38 stars 18 forks source link

[Bug] Quantity creation #167

Closed mwiesiolek closed 7 years ago

mwiesiolek commented 7 years ago

Hi,

while executing the following code:

Quantities.getQuantity("20 1/K");

I receive: 

java.lang.IllegalArgumentException: unexpected token 8

    at tec.uom.se.quantity.Quantities.getQuantity(Quantities.java:80)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
    at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:237)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

My dependencies versions are:

    compile("tec.uom:uom-se:1.0.7")
    compile("tec.uom.lib:uom-lib:1.0.2")
    compile("systems.uom:systems-common-java8:0.7.1")

Thanks in advance for helping/fixing it

Regards, Maks

keilw commented 7 years ago

What Unit is 1/K supposed to represent? The parser (at least in SimpleUnitFormat) won't allow a digit as start of a unit because it does not know wheter it's still part of the numeric element or not. We have to explore how e.g. EBNFUnitFormat deals with such special cases. Did you try that?

Right now using EBNFUnitFormat in QuantityFormat requires to compose them, but you can do that via

QuantityFormat.getInstance(NumberFormat.getInstance(), EBNFUnitFormat.getInstance());

And then call parse().

mwiesiolek commented 7 years ago

It represents linear coefficient of thermal expansion: https://en.wikipedia.org/wiki/Thermal_expansion

Let me test your proposal. I will let you know about my outcome.

mwiesiolek commented 7 years ago

Quick update:

I've executed test which looks as following:

    @Test
    public void test(){

        // when
        ComparableQuantity<?> quantity = QuantityFormat.getInstance(NumberFormat.getInstance(), EBNFUnitFormat.getInstance()).parse("20 1/K");

        // then
        assertThat(quantity, notNullValue());
        assertThat(quantity.getValue().doubleValue(), is(20.0));
    }

and this happens:

javax.measure.format.ParserException: tec.uom.se.internal.format.TokenMgrError: Lexical error at line 1, column 3.  Encountered: " " (32), after : ""

    at tec.uom.se.format.EBNFUnitFormat.parse(EBNFUnitFormat.java:269)
    at tec.uom.se.format.EBNFUnitFormat.parse(EBNFUnitFormat.java:274)
    at tec.uom.se.format.NumberSpaceQuantityFormat.parse(NumberSpaceQuantityFormat.java:102)
    at tec.uom.se.format.NumberSpaceQuantityFormat.parse(NumberSpaceQuantityFormat.java:108)
    at tec.uom.se.format.NumberSpaceQuantityFormat.parse(NumberSpaceQuantityFormat.java:113)
    at com.matmatch.core.converter.QuantityConverterTest.test(QuantityConverterTest.java:84)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
    at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:237)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: tec.uom.se.internal.format.TokenMgrError: Lexical error at line 1, column 3.  Encountered: " " (32), after : ""
    at tec.uom.se.internal.format.UnitTokenManager.getNextToken(UnitTokenManager.java:433)
    at tec.uom.se.internal.format.UnitFormatParser.jj_scan_token(UnitFormatParser.java:728)
    at tec.uom.se.internal.format.UnitFormatParser.jj_3R_4(UnitFormatParser.java:581)
    at tec.uom.se.internal.format.UnitFormatParser.jj_3_1(UnitFormatParser.java:575)
    at tec.uom.se.internal.format.UnitFormatParser.jj_2_1(UnitFormatParser.java:528)
    at tec.uom.se.internal.format.UnitFormatParser.addExpr(UnitFormatParser.java:102)
    at tec.uom.se.internal.format.UnitFormatParser.compoundExpr(UnitFormatParser.java:77)
    at tec.uom.se.internal.format.UnitFormatParser.parseUnit(UnitFormatParser.java:65)
    at tec.uom.se.format.EBNFUnitFormat.parse(EBNFUnitFormat.java:257)
    ... 27 more

I will devote more time later on in order to investigate this

keilw commented 7 years ago

It probably also violates the EBNF definitions used by EBNFUnitFormat. UCUM is based on a very similar BNF: http://unitsofmeasure.org/ucum.html#section-Syntax-Rules And the only case I know where "1/something" is applied would be Hertz. Being "s^-1". uom-se declares it as ONE.divide(SECOND) but note ONE must not be labeled or aliased with "1" for the same parsing reasons mentioned earlier. I am fairly confident "one/K" could work in this case, but unless the exception is caused by something different I cannot guarantee that 1 or another digit in front of a unit name or label used to parse and identify will work for either of the systems.

keilw commented 7 years ago

UCUMFormat allows doing this, see https://github.com/unitsofmeasurement/jackson-module-unitsofmeasure/blob/master/src/test/java/com/opower/unitsofmeasure/TestUnitJacksonModule.java (testParseTemperatureInverse()), so if you put UCUMFormat.getInstance(CASE_INSENSITIVE) in place of EBNFUnitFormat, the sample code should pass. Please test, it requires adding https://github.com/unitsofmeasurement/uom-systems/tree/master/ucum-java8 as dependency, but it's based on uom-se and the Java SE 8 stack. If that works, for now, this is the only UnitFormat implementation for those kinds of operations. I can't predict or promise for sure, but the way UCUMFormat works there is a chance, EBNFUnitFormat might also allow it in a future version, but a lot of it is based on generated code, so can't say for sure. SimpleUnitFormat as the name says is much more simple, so it likely never will allow certain things like a digit in the first position of a unit label.

keilw commented 7 years ago

@mwiesiolek Hi, I closed this as UCUMFormat works for your use case. If you have a problem with that, please raise an issue in unit-systems ideally. https://github.com/unitsofmeasurement/uom-se/issues/145 also still exists here and whatever EBNFUnitFormat may use from either LocalUnitFormat or UCUMFormat, we'll try to factor in and make available.

mwiesiolek commented 7 years ago

@keilw sure, thank you for keeping me in the loop. I haven't given you any input since I did not have time :), sorry for that.

I will test your suggestion and let you know whether or not it works in my case.

mwiesiolek commented 7 years ago

Hi @keilw,

So, I've executed the following code:

UCUMFormat.getInstance(UCUMFormat.Variant.CASE_INSENSITIVE).parse("20 1/K");

and I receive exception which looks as:

systems.uom.ucum.internal.format.TokenMgrError: Lexical error at line 1, column 3.  Encountered: " " (32), after : ""

    at systems.uom.ucum.internal.format.UCUMTokenManager.getNextToken(UCUMTokenManager.java:412)
    at systems.uom.ucum.internal.format.UCUMFormatParser.jj_ntk(UCUMFormatParser.java:509)
    at systems.uom.ucum.internal.format.UCUMFormatParser.Term(UCUMFormatParser.java:75)
    at systems.uom.ucum.internal.format.UCUMFormatParser.parseUnit(UCUMFormatParser.java:62)
    at systems.uom.ucum.format.UCUMFormat$Parsing.parse(UCUMFormat.java:472)
    at systems.uom.ucum.format.UCUMFormat$Parsing.parse(UCUMFormat.java:490)

It seems that UCUMFormat does not work as well.

keilw commented 7 years ago

No, it works well, but like all other UnitFormat implementations, UCUMFormat is not meant for a Quantity by itself. I proposed to change that in a future version of the API https://github.com/unitsofmeasurement/unit-api/issues/47, for now, you have to use the implementation class. https://github.com/unitsofmeasurement/uom-systems/blob/master/ucum-java8/src/test/java/systems/uom/ucum/format/UnitFormatTest.java includes both testParseUCUMCITemperatureInverse() and testParseUCUMCSTemperatureInverse() for each parser.