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

Cannot convert Celcius to canonical Form #26

Closed
epicadk opened this issue Oct 30, 2021 · 9 comments
Closed

Cannot convert Celcius to canonical Form #26

epicadk opened this issue Oct 30, 2021 · 9 comments

Comments

@epicadk
Copy link

epicadk commented Oct 30, 2021

Trying to run getCanonicalForm() on 100 Cel returns a null value with the code K

@michaelabuckley
Copy link

[degF] doesn't work either.

@grahamegrieve
Copy link
Contributor

what would a test case look like?

@delcroip
Copy link

kg, years, day, month are working

@delcroip
Copy link

g/L and alike (mg/dL) and K are not working

@khyativyasargus
Copy link

Test case Reference

@grahamegrieve
Copy link
Contributor

Added a test case that checks that this conversion fails with a UCUM exception. Actually supporting this test cases requires a total rework of the library architecture (offsets are supper painful)

@jingtang10
Copy link

@grahamegrieve is there a tracking issue for the work?

@khyativyasargus I think we can put in a workaround in the android fhir sdk repo for now until this is fixed/released in the ucum lib. I commented on the issue in the sdk repo.

@grahamegrieve
Copy link
Contributor

is there a tracking issue for the work?

no, because I'm not sure I'm going to do it. It's relatively easy to think that the library should be able to convert between F and C directly, but harder to figure out what to do in general. Consider say mm/C vs mm/F - that doesn't get an offset conversion. So what about C/mm vs F/mm - neither does that. but what about mm/C2 convert to mm/F2? Is that even a thing? I'm not sure.

so maybe I could just write a special case just for F vs C?

@jingtang10
Copy link

i'm not great at physics, but i'd imagine when it comes to compound units it should be kelvin instead of C or F? I don't really understand mm/C or mm/F or C/mm or F/mm.

the original issue is about getting canonical form of C in K, because we do comparison in canonical units in the library. we don't really need F <-> C.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants