Stream: fhirpath
Topic: Date Time operations
Grahame Grieve (Sep 22 2020 at 21:12):
@Bryn Rhodes @Ewout Kramer please review this:
https://github.com/FHIR/fhir-test-cases/pull/76/files
@Keith Boone it seems to me that should add more tests than this - at a minimum, UCUM coded times, decimal values in the quantities, and all the duration units...
Keith Boone (Sep 22 2020 at 23:51):
What, you want both code and testing from the same developer? Is the some sort of open source project? Oh yeah, duh.
Yes. I’ll put it on my todo list unless someone beats me to it. I’m curious though, what is 0.5 of a month or year equal to? Sub integer precision works for any unit smaller than a month (with minor accuraccy lost due to leap seconds maybe, depending on what java does and how you account for it). For wuick thinks, I usually just work with 30 and 365 days, but whole unit months and years are expected to preserve dates I thought, at least wrt addition/subtraction.
Sorry about white space, I have to go fix how my tab settings are set up in eclipse.
Grahame Grieve (Sep 22 2020 at 23:54):
right. so this is why I want tests, because there's lots of challenges here
Bryn Rhodes (Sep 23 2020 at 02:13):
In FHIRPath, date/time arithmetic above weeks is strictly calendar-based, fractions of durations are dropped before the calculation is performed.
Bryn Rhodes (Sep 23 2020 at 02:19):
Agree with the test outcomes
Bryn Rhodes (Sep 23 2020 at 02:20):
My engine gives the same result for all 4 tests
Grahame Grieve (Sep 23 2020 at 06:52):
just silently dropped?
Bryn Rhodes (Sep 23 2020 at 21:42):
The CQL engine (as encouraged by the spec) issues a warning, but otherwise yes.
Grahame Grieve (Sep 29 2020 at 02:01):
@Bryn Rhodes @Ewout Kramer extending these tests, do you agree with all of these:
@1973-12-25 + 7 'days' = 1974-01-01
@1973-12-25 + 7.7 'days' = 1974-01-01
@1973-12-25T00:00:00.000+10:00 + 7 'days' = 1974-01-01T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 7.7 'days' = 1974-01-01T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 's' = 1973-12-25T00:00:01.000+10:00
@1973-12-25T00:00:00.000+10:00 + 0.1 's' = 1973-12-25T00:00:00.100+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'min' = 1973-12-25T00:01:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'h' = 1973-12-25T01:00:00.000+10:00
@1973-12-25 + 1 'd' = 1973-12-26
@1973-12-25 + 1 'mo' = 1974-01-01
@1973-12-25 + 1 'wk' = 1975-01-01
@1973-12-25 + 1 'a' = 1974-12-24
@1973-12-25T00:00:00.000+10:00 + 1 'second' = 1973-12-25T00:00:01.000+10:00
@1973-12-25T00:00:00.000+10:00 + 0.1 'second' = 1973-12-25T00:00:00.100+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'minute' = 1973-12-25T00:01:00.000+10:00 + 1
@1973-12-25T00:00:00.000+10:00 + 1 'hour' = 1973-12-25T01:00:00.000+10:00
@1973-12-25 + 1 'day' = 1973-12-26
@1973-12-25 + 1 'month' = 1974-01-01
@1973-12-25 + 1 'week' = 1975-01-01
@1973-12-25 + 1 'year' = 1974-12-25
```
Brian Postlethwaite (Sep 29 2020 at 02:20):
.7 days goes missing?
Grahame Grieve (Sep 29 2020 at 02:20):
yes see discussion above
Bryn Rhodes (Sep 29 2020 at 03:06):
Here are the results from the Java-based engine:
define Test1: @1973-12-25 + 7 'days' = @1974-01-01
define Test2: @1973-12-25 + 7.7 'days' = @1974-01-01
define Test3: @1973-12-25T00:00:00.000+10:00 + 7 'days' = @1974-01-01T00:00:00.000+10:00
define Test4: @1973-12-25T00:00:00.000+10:00 + 7.7 'days' = @1974-01-01T00:00:00.000+10:00
define Test5: @1973-12-25T00:00:00.000+10:00 + 1 's' = @1973-12-25T00:00:01.000+10:00
define Test6: @1973-12-25T00:00:00.000+10:00 + 0.1 's' = @1973-12-25T00:00:00.100+10:00
define Test7: @1973-12-25T00:00:00.000+10:00 + 1 'min' = @1973-12-25T00:01:00.000+10:00
define Test8: @1973-12-25T00:00:00.000+10:00 + 1 'h' = @1973-12-25T01:00:00.000+10:00
define Test9: @1973-12-25 + 1 'd' = @1973-12-26
define Test10: @1973-12-25 + 1 'mo' = @1974-01-01 // This should be a run-time error
define Test11: @1973-12-25 + 1 'wk'// = @1974-01-01
define Test12: @1973-12-25 + 1 'a' = @1974-12-24 // This should be a run-time error
define Test13: @1973-12-25T00:00:00.000+10:00 + 1 'second' = @1973-12-25T00:00:01.000+10:00
define Test14: @1973-12-25T00:00:00.000+10:00 + 0.1 'second' = @1973-12-25T00:00:00.100+10:00
define Test15: @1973-12-25T00:00:00.000+10:00 + 1 'minute' = @1973-12-25T00:01:00.000+10:00
define Test16: @1973-12-25T00:00:00.000+10:00 + 1 'hour' = @1973-12-25T01:00:00.000+10:00
define Test17: @1973-12-25 + 1 'day' = @1973-12-26
define Test18: @1973-12-25 + 1 'month'// = @1974-01-25
define Test19: @1973-12-25 + 1 'week'// = @1974-01-01
define Test20: @1973-12-25 + 1 'year' = @1974-12-25
Bryn Rhodes (Sep 29 2020 at 03:09):
Test11, for example, should be 1974-01-01, not 1975-01-01, right?
Grahame Grieve (Sep 29 2020 at 03:15):
umm yeah I have a bunch of updates shortly
Bryn Rhodes (Sep 29 2020 at 03:20):
(y)
Grahame Grieve (Sep 29 2020 at 03:28):
after actually implementing them, I get these:
@1973-12-25 + 7 'days' = @1974-01-01
@1973-12-25 + 7.7 'days' = @1974-01-01
@1973-12-25T00:00:00.000+10:00 + 7 'days' = @1974-01-01T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 7.7 'days' = @1974-01-01T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'sec' = @1973-12-25T00:00:01.000+10:00
@1973-12-25T00:00:00.000+10:00 + 0.1 'sec' = @1973-12-25T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 10 'msec' = @1973-12-25T00:00:00.010+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'min' = @1973-12-25T00:01:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'h' = @1973-12-25T01:00:00.000+10:00
@1973-12-25 + 1 'd' = @1973-12-26
@1973-12-25 + 1 'mo' = @1974-01-23
@1973-12-25 + 1 'wk' = @1974-01-01
@1973-12-25 + 1 'a' = @1974-12-25
@1975-12-25 + 1 'a' = @1976-12-24
@1973-12-25T00:00:00.000+10:00 + 1 'second' = @1973-12-25T00:00:01.000+10:00
@1973-12-25T00:00:00.000+10:00 + 10 'millisecond' = @1973-12-25T00:00:00.010+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'minute' = @1973-12-25T00:01:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'hour' = @1973-12-25T01:00:00.000+10:00
@1973-12-25 + 1 'day' = @1973-12-26
@1973-12-25 + 1 'month' = @1974-01-25
@1973-12-25 + 1 'week' = @1974-01-01
@1973-12-25 + 1 'year' = @1974-12-25
Ewout Kramer (Sep 29 2020 at 13:27):
@1973-12-25 + 1 'mo' = @1974-01-23
I am getting 1974-01-24. I am adding 365.24/12 days. Is that wrong?
Ewout Kramer (Sep 29 2020 at 13:29):
And I keep on getting confused with the exact literals for dates. I am pretty sure that UCUM units go between quotes, but our custom calender units not, so, according to what I see here: http://hl7.org/fhirpath/#addition-2
This test @1973-12-25 + 1 'month' = @1974-01-25 should look like @1973-12-25 + 1 month = @1974-01-25, right?
Bryn Rhodes (Sep 29 2020 at 13:32):
Based on the date/time arithmetic topic, @1973-12-25 + 1 'mo' should give an error, on the grounds that it's using a definite-time duration unit as a calendar duration.
Bryn Rhodes (Sep 29 2020 at 13:35):
On whether the calendar duration units are supposed to be quoted, the narrative for Quantity allows for both, though all the examples show calendar duration keywords without quotes, and the grammar supports the keywords directly.
Ewout Kramer (Sep 29 2020 at 13:37):
Ok, then I meant @1973-12-15 + 1 month. In my 365.25/4 addition that gives 1974-01-24 (10:30 in the morning)
Ewout Kramer (Sep 29 2020 at 13:38):
If we conflate x month and x 'month', you'll never get an error that "month" is not a valid UCUM code, right? So anything between the quotes can be either UCUM or a calender duration? What does (2 minutes).ToString() print in your implementation?
Ewout Kramer (Sep 29 2020 at 13:39):
The same as 2 'minutes' and 2 'min'?
Bryn Rhodes (Sep 29 2020 at 13:39):
It's calendar-based, so you actually just add 1 month, and get @1974-01-15.
Ewout Kramer (Sep 29 2020 at 13:40):
oh right. yes, that would happen if I would have implemented it.
Bryn Rhodes (Sep 29 2020 at 13:41):
(2 minutes).toString() gives 2 'minutes' on the Java-based engine
Ewout Kramer (Sep 29 2020 at 13:42):
That surprises me.
Ewout Kramer (Sep 29 2020 at 13:44):
But some of the recent discussions have confused me thoroughly here. I think I understood that year/month are still calender units, separate from UCUM, but that minutes/weeks/days etc are aligned with UCUM min,wk,d. Is that correct?
Bryn Rhodes (Sep 29 2020 at 13:45):
Yes, we have relaxed the "equivalent to UCUM" boundary to days (and weeks), rather than seconds, on the grounds that considering leap seconds is not generally a practical problem.
Ewout Kramer (Sep 29 2020 at 13:45):
Ok, so if my engine does (1 week).ToString() = 1 'wk', that's fine?
Ewout Kramer (Sep 29 2020 at 13:46):
Or am I normalizing to UCUM too eagerly here
Bryn Rhodes (Sep 29 2020 at 13:48):
That expression should be true, yes, but whether you want to be able to round-trip there is another question. The specs (FHIRPath and CQL) don't require the round-trip.
Bryn Rhodes (Sep 29 2020 at 13:48):
Though we have been trying to get consistent behavior there.
Ewout Kramer (Sep 29 2020 at 13:49):
Yeah, I can see you're storing the actual literal, I guess the java implementation will do 1 months.ToString() = 1 'months' (so we do normalize to within quotes?)
Bryn Rhodes (Sep 29 2020 at 14:02):
Once it's the unit element of a Quantity value, it's just a string, so any processing already has to deal with the fact that it might be a UCUM code, and it might be a calendar duration keyword. So I don't know that we intentionally implemented it to support both, but rather that the most natural implementation supported both as a by-product.
Grahame Grieve (Sep 29 2020 at 19:52):
I admit to being confused about the calendar periods in the units when I made up the tests
Grahame Grieve (Sep 29 2020 at 19:58):
the reason it's confusing is this table:
Grahame Grieve (Sep 29 2020 at 19:58):
Grahame Grieve (Sep 29 2020 at 19:58):
you see that the unit representation has ' in the column
Bryn Rhodes (Sep 29 2020 at 20:54):
Yes, I can see that that table is confusing, it was intended to distinguish between the keyword and the resulting value of the unit element of the Quantity. The Java implementation just turns the keyword into the string representation, so the difference just disappears.
Bryn Rhodes (Sep 29 2020 at 20:55):
Does your parser accept the keyword version?
Grahame Grieve (Sep 29 2020 at 21:00):
yes it accepts either. I updated the tests - just committing now. Here's the modified version
Grahame Grieve (Sep 29 2020 at 21:02):
@1973-12-25 + 7 days = @1974-01-01
@1973-12-25 + 7.7 days = @1974-01-01
@1973-12-25T00:00:00.000+10:00 + 7 days @1974-01-01T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 7.7 days @1974-01-01T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 second @1973-12-25T00:00:01.000+10:00
@1973-12-25T00:00:00.000+10:00 + 10 millisecond @1973-12-25T00:00:00.010+10:00
@1973-12-25T00:00:00.000+10:00 + 1 minute @1973-12-25T00:01:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 hour @1973-12-25T01:00:00.000+10:00
@1973-12-25 + 1 day = @1973-12-26
@1973-12-25 + 1 month = @1974-01-25
@1973-12-25 + 1 week = @1974-01-01
@1973-12-25 + 1 year = @1974-12-25
@1973-12-25 + 1 'd' = @1973-12-26
@1973-12-25 + 1 'mo' -> error
@1973-12-25 + 1 'wk' = @1974-01-01
@1973-12-25 + 1 'a' -> error
@1975-12-25 + 1 'a' -> error
@1973-12-25T00:00:00.000+10:00 + 1 's' @1973-12-25T00:00:01.000+10:00
@1973-12-25T00:00:00.000+10:00 + 0.1 's' @1973-12-25T00:00:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 10 'ms' @1973-12-25T00:00:00.010+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'min' @1973-12-25T00:01:00.000+10:00
@1973-12-25T00:00:00.000+10:00 + 1 'h' @1973-12-25T01:00:00.000+10:00
@1974-12-25 + 7 -> error
Paul Lynch (Sep 30 2020 at 16:47):
"If a definite-quantity duration above seconds appears in a date/time arithmetic calculation, the evaluation will end and signal an error to the calling environment." (http://hl7.org/fhirpath/#datetime-arithmetic). Shouldn't + 1 'd' , + 1 'wk', + 1 'min', and + 1 'h' result in errors, or is that behavior changing?
Paul Lynch (Sep 30 2020 at 16:59):
Never mind-- I see that these are tests for a change to the specification. https://chat.fhir.org/#narrow/stream/179266-fhirpath/topic/Date.2FTime.20comparison.20vs.20equality/near/209114649
Bryn Rhodes (Oct 02 2020 at 20:24):
So one thing that we've just noticed in getting these tests to all pass is that we are using the code element of the FHIR Quantity, which means that we need to set the system element as well. So two questions arise:
- Are all the engines using
Quantity.codehere, or are some of them usingunit? - The
systemishttp://unitsofmeasure.orgfor the UCUM units, but we don't actually have a system for the calendar duration units, what should thesystemvalue be when thecodeis a calendar duration unit? I proposehttp://hl7.org/fhirpath/CodeSystem/calendar-units
Bryn Rhodes (Oct 02 2020 at 20:26):
@Grahame Grieve , @Ewout Kramer , @Paul Lynch , @Chris Moesel , @Lee Surprenant
Chris Moesel (Oct 02 2020 at 20:35):
I'm coming in late to the conversation. Are we talking about how we convert 1 'wk' and 1 week to instances of FHIR Quantity (for the purpose of comparison)?
Chris Moesel (Oct 02 2020 at 20:36):
That's not really a concern in my engine because we're always going the other way (converting FHIR Quantity to CQL Quantity for doing comparisons). But I'm cool w/ what you propose above if it's useful for FP implementations.
Paul Lynch (Oct 02 2020 at 21:11):
When fhirpath.js parses 1 'wk' it creates an internal FP_Quantity instance which does not bother storing the system, because the unit is either UCUM or one of the calendar durations, and the case easily determined when needed.
Paul Lynch (Oct 02 2020 at 21:16):
To answer the specific questions from Bryn:
1) We're really not using FHIR Quantity for that. FP_Quantity puts the unit in "unit", not "code" (and in fact does not have a "code").
2) I have no objection to creating a code system for the calendar duration units, and the the URI looks fine to me.
Grahame Grieve (Oct 02 2020 at 23:48):
I'm using unit+system+code for UCUM units, and just unit for the other informal FHIRPath defined units, and doing magic on them. I agree that it would be more natural to us a defined system internally, and I'd happily use an agreed one, but I think it's all entirely internal details unless you propose for using the formal definition in other Quantities, which is not a FHIRPath issue
Bryn Rhodes (Oct 03 2020 at 16:08):
Having that be an internal implementation detail makes complete sense to me, and I'm also hearing support for the idea of defining the code system, so I've submitted a tracker to that effect: https://jira.hl7.org/browse/FHIR-28927
Last updated: Apr 12 2022 at 19:14 UTC
