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

EBNFUnitFormat can not handle chain of TransformedUnit #406

Open
unixfan2 opened this issue Nov 28, 2023 · 4 comments
Open

EBNFUnitFormat can not handle chain of TransformedUnit #406

unixfan2 opened this issue Nov 28, 2023 · 4 comments

Comments

@unixfan2
Copy link

val formatter = DefaultFormatService().getUnitFormat("EBNF")
val test = Units.METRE.multiply(2).multiply(2)
println(formatter.format(test))

result: m·2

I guess the reason for that is that EBNFHelper.formatInternal is using "parentUnit = unit.getSystemUnit()" instead of the direct parent in the last else block for handling transformed units.
Maybe changing that would also make all the dirty hacks in that section obsolete.

@unixfan2
Copy link
Author

Please note that the local formatter is implemented similarly. Maybe some redundant stuff can be merged.

@keilw
Copy link
Member

keilw commented Nov 29, 2023

Thanks, could you try, how UCUMFormat treats this as a comparison?

@unixfan2
Copy link
Author

val indriyaFormatter = DefaultFormatService().getUnitFormat("EBNF")
val ucumFormatter = UCUMFormatService().unitFormat
val indriyaUnit = Units.METRE.multiply(2).multiply(2)
val ucumUnit = UCUM.METER.multiply(2).multiply(2)
println("indriyaFormatter/indriyaUnit:" + indriyaFormatter.format(indriyaUnit))
println("ucumFormatter/indriyaUnit:" + ucumFormatter.format(indriyaUnit))
println("indriyaFormatter/ucumUnit: " + indriyaFormatter.format(ucumUnit))
println("ucumFormatter/ucumUnit: " + ucumFormatter.format(ucumUnit))

Result:

indriyaFormatter/indriyaUnit:m·2
ucumFormatter/indriyaUnit:(m.2).2
indriyaFormatter/ucumUnit: m·2
ucumFormatter/ucumUnit: (m.2).2

The UCUM formatter seems to handle that nicely.

I tried my suggested fix for the EBNF stuff and it works fine in my initial tests, but I'm too lazy to apply for a JCP/JSR membership just for creating a PR. But my fix is only to set the parentUnit to the parent unit in the case of a transformed unit + deleting the workaround stuff in that section (hopefully there are unit tests which cover them, but the unit tests work as without the fix).

@keilw
Copy link
Member

keilw commented Nov 29, 2023

Thanks for trying. As they all use resource bundles underneath the hood, there could be some inspiration or patch based on UCUMFormat. The latter does not need JCP membership. For the odd PR I also think that is fine, and you could propose one without a JCP membership. If someone contributes more often, or many lines of code, then it would be better.

@keilw keilw added this to the 2.2.1 milestone Nov 29, 2023
@keilw keilw modified the milestones: 2.2.1, 2.2.2 Aug 19, 2024
@keilw keilw modified the milestones: 2.2.2, 2.2.3 Dec 8, 2024
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

2 participants