Unit normalization fixes for ucum and calendar based cql quantity #1025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ions retrieve Display while generating FHIR components
…ract operation should not be allowed, as it could not add x day to 9999/12/31 and subtract x day from 01/01/01
…alendar. Ucum follows fixed quantity calculations. Fixed logic to allow both units and calculate accordingly. Updated/Added unit tests.
…endar-based-CqlQuantity
|
This PR should either be in draft, or if it is ready to be reviewed, assigned to a reviewer. I assume it's ready, so I'll assign myself this time |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes unit normalization issues for UCUM and calendar-based CQL quantities by replacing UCUM unit codes with CQL calendar units. The changes eliminate automatic conversion between CQL units and UCUM codes, allowing both to coexist while maintaining backward compatibility.
Key changes:
- Replaced UCUM unit mapping logic with direct CQL unit handling
- Updated arithmetic operators to handle both singular/plural forms of units
- Modified date/time precision handling to use CQL calendar units instead of UCUM codes
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
Cql/Iso8601/DateTimePrecision.cs |
Updates unit parsing to handle full unit names instead of UCUM character codes |
Cql/Cql.Runtime/Operators/CqlOperators.TypeOperators.cs |
Removes UCUM unit conversion in normalize operator |
Cql/Cql.Runtime/Operators/CqlOperators.ListOperators.cs |
Replaces UCUM unit references with CQL units in expand operations |
Cql/Cql.Runtime/Operators/CqlOperators.IntervalOperators.cs |
Updates interval expansion to use CQL units instead of UCUM |
Cql/Cql.Runtime/Operators/CqlOperators.DateTimeOperators.cs |
Removes UCUM unit conversion in precision comparison |
Cql/Cql.Runtime/Operators/CqlOperators.ArithmeticOperators.cs |
Adds normalization logic for singular/plural unit forms |
Cql/Cql.Runtime/Conversion/UnitConverter.cs |
Removes UCUM unit conversion and fixes typo |
Cql/Cql.Runtime/Conversion/UcumConversionExtensions.cs |
Fixes typo in comment |
Cql/Cql.Runtime/Conversion/ConversionConstants.cs |
Updates days per year to include leap year calculation |
Cql/Cql.Runtime/Comparers/CqlComparers.CqlQuantityCqlComparer.cs |
Fixes grammar in comment |
Cql/Cql.Abstractions/Primitives/CqlTime.cs |
Updates unit handling to use full unit names instead of character parsing |
Cql/Cql.Abstractions/Primitives/CqlQuantity.cs |
Removes automatic UCUM unit conversion |
Cql/Cql.Abstractions/Primitives/CqlDateTimeMath.cs |
Updates precision handling to use CQL units |
Cql/Cql.Abstractions/Primitives/CqlDateTime.cs |
Updates unit handling for date/time arithmetic operations |
Cql/Cql.Abstractions/Primitives/CqlDate.cs |
Updates unit handling for date arithmetic operations |
Cql/Cql.Abstractions/Abstractions/Units.cs |
Replaces UCUM mapping with date precision to CQL unit mapping |
Cql/Cql.Abstractions/Abstractions/UCUMUnits.cs |
Adds date/time unit constants and removes precision mapping function |
Cql/CoreTests/PrimitiveTests.cs |
Updates tests to reflect new unit behavior |
Cql/CoreTests/ModelTest.cs |
Updates test to use CQL unit instead of UCUM |
|
I agree with copilot's remarks |
…endar-based-CqlQuantity
…endar-based-CqlQuantity
…endar-based-CqlQuantity
…endar-based-CqlQuantity
Co-authored-by: Copilot <[email protected]>
…lQuantity' of https://github.com/ncqa-org/firely-cql-sdk-v2.0-hedis-test into Unit-normalization-fixes-for-UCUM-and-Calendar-based-CqlQuantity
resolved comments. Also resolved other build errors that I reproduced with Release config, however, still pipeline build fails and I have no clue what's causing it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1024