Feature Request: Partial Date-Time objects

[billdenney]

The purpose of this issue is to discuss the needs of the partial date-time object from both a user perspective (where the users for the object would be focused on admiral developers-- not admiral users) and from a developer perspective (where the developers would be developers of the partial date-time object code).

(This didn't seem to fit within the other issue templates. It's similar to a feature request, but it's a bit wider-ranging.)

This is related to #1300 (and a bit related to #1299). It is also related to tidyverse/lubridate#690 (see that issue for discussion of some of the challenges and suggestions on object formatting and needs).

@dgkf, I was just reminded about your parttime package which has a lot of overlap with this (https://github.com/dgkf/parttime). With a quick look, I think that parttime supports decreased precision but not missing-in-the-middle formats (for example, "2022--13" for missing month would not be supported). Is that correct? Might you be interested in collaborating on this?

As discussed in the meeting a few minutes ago, there is a desire to have partial date-time objects that would be able to handle the ISO 8601 variant that is used across CDISC standards. The final location of the object may either be in admiral or in a new package. (CDISCpartialdate? the package name would need consideration.)

My initial thoughts on the object are:

• There would be variants of the object for date-time, date, and time.
• The object would contain year, month, day, hour, minute, second, and timezone (as appropriate for the object type).
• The object would be fully vectorized. Notably, timezone may differ by element which is not generally supported across R.
• The object would store its default maximum precision.
  • For example, if it is parsed like lubridate::ymd_hm(x), the default maximum precision would be minutes.
  • For example, if it is parsed like lubridate::ymd_hms(x, truncated=1), the default maximum precision would be seconds but they may be missing.
  • In the real code, we would not use lubridate parsing because that cannot handle missing parts in the middle. The above examples are to show how it may look from a user perspective.
• The object must be able to roundtrip from character to object to character without loss of information or modification.

My initial thoughts on the code are:

• It would need to be able to parse from character strings following the full CDISC standard which is a variant of ISO 8601.
  • The initial version would focus on dates, times, and date-times. Future versions may add periods and durations.
• Parsing would need to have sufficiently-high performance.
  • My initial guess for the performance requirement would be >10,000 dates-times parsed/second (and >100,000/second would be a goal) when passed in as a vector
• It would need getters and setters for each part: year, month, day, hour, minute, second, and time zone.
  • When year, month, day, hour, or minute is not set, it should return NA_integer_; missing second would return NA_real_; missing timezone would return NA_character_. (Alternative: Should it return NA_real_ for any part except timezone?)
• It would need to support at minimum the following additional operations:
  • Indicate the maximum full-precision. For example, if year was given, month was missing, and day was given: The maximum full precision would be year.
  • Indicate the missing parts. For example, if year was given, month was missing, and day was given: The missing part would be month if it is a partial-date object or month, hour, minute, second, and timezone if it is date-time object.

Additional thoughts?

[dgkf]

Thanks for the tag, @billdenney

I think parttime would address almost all of these requirements.

I haven't benchmarked the performance, but I was very conscious of trying to make it vectorized in all cases - it's held up to some pretty large datasets. Also, as you highlighted, it does not handle missing-in-the-middle scenarios (some quick tests below). For imputation alone, I think missing-in-the-middle would be a pretty minor update. But right now, anytime you do datetime arithmetic, any coarse uncertainty will cascade so missing-in-the-middle won't be preserved. The desired behavior in these cases isn't totally clear to me, though.

Let me know what use cases you had in mind, I think it would be a fairly minimal change.

---

A quick test, and with a very minor change to the parsing regex, the parser will accept missing-in-the-middle strings. This will be fine for parsing and imputation of missing-in-the-middle:

> as.parttime("2022--13")
<partial_time<YMDhmsZ>[1]>
[1] "2022-NA-13"

> impute_time(as.parttime("2022--13"), "2022-07-13")
<partial_time<YMDhms+tz>[1]>
[1] "2022-07-13"

But comparison operators don't always work:

> as.parttime("2022--04") > as.parttime("2022-01-05")
[1] NA
> as.parttime("2022--04") > as.parttime("2022-01-03")
[1] TRUE
> as.parttime("2022--04") > as.parttime("2022-12-07")
[1] NA  # this should be decidedly FALSE

I'd have to review the spec, but I don't think these are valid ISO8601 datetimes anymore, so I would prefer to keep the default behavior as is and allow a format = "partial-iso8601" parameter to as.parttime to use an alternative parser that is more permissive

[billdenney]

@dgkf Thanks!

I had a typo in my example date. SDTM requires a dash (-) for missing date-time parts. So the example I gave would be 2022---13 (with another dash indicating the missing month in the middle).

I would not expect parttime to support the nonstandard CDISC format as its core method. Having a format argument added would make sense to me.

Does parttime support dates and times in addition to date-times? (I don't see examples with only one or the other part; I only see full date-times.)

[dgkf]

Does parttime support dates and times in addition to date-times? (I don't see examples with only one or the other part; I only see full date-times.)

Date types

Dates are considered a subset of datetimes - just a datetime where the time components are missing. This has a bit of overhead because a matrix is still allocated which just contains NAs for all the time components. Practically, I imagine this overhead is pretty inconsequential, but please let me know if there's situations where having a lighter-weight date-only structure would be necessary.

Time types

Times can be provided as well, but like the "missing-in-the-middle" case, the logic isn't really set up to handle situations where only a time is known. There was no intentional omission - the package just started off with a fairly specific use case in mind and this wasn't considered. I think it could be extended to cover these cases and repurpose a lot of the datetime logic to support times.

If I were to go down that path, I think a standalone time should be treated as a timespan. That is, instead of interpreting "01:59pm" as "1:59pm" with a missing day, it generalizes more nicely to think of it as a duration of "13h 59s", which might have specific meaning if you know it is meant to represent a time on a day. This way it isn't a datetime with a missing date, but a duration with some amount of precision. Perhaps there could be different subclasses just for the sake of default formatting, but under the hood they'd be represented as timespans.

[billdenney]

I'm thinking of dates and times a bit differently than date-times. The reason that I think of them differently is because the inherent missingness is different.

Specifically, I'd rather not have the max of a date of "2022-07-13" be "2022-07-13T23:59:59" but for it to remain "2022-07-13". And, similarly, I'd want times to be handled in a way where the date-part is not considered missing but nonexistent.

One way to possibly make them work within the current parttime having near-zero modification could be to have some fixed reference date without missingness for times (so internally, it would be represented like "0001-01-01Thh:mm:ss" for times, for instance).

[dgkf]

Specifically, I'd rather not have the max of a date of "2022-07-13" be "2022-07-13T23:59:59" but for it to remain "2022-07-13".

I see - I think my preference would still be to try to wrangle this into the more general case. One mechanism might be to offer an imputation function that only imputes maximums to a specific resolution (in this case only to the "date") field, and make a specialized impute_date_max function.

But I think we might be going off on a tangent here. I filed a separate issue dgkf/parttime#10 to think about this specific example. Feel free to open other issues/feature requests if there are other scenarios that might require a separate type.

[billdenney]

That partial imputation would align well with the highest_imputation argument proposed in #1300.

[bundfussr]

For admiral we need a partialdate and a partialdatetime object. A partialtime object is not required because a time variable is usually derived from the imputed datetime but not from the DTC variable.

For both objects we need getters and setters for each component and a method which returns the object as ISO8601 character string. I think these are the minimum requirements for working on #1300.

Helpful methods for the objects would be:

• Methods to determine the range of the dates/datetimes represented by the object, i.e., minimum and maximum date or datetime. These methods could return a numeric date/datetime value.
• A method returning the highest missing component. It could return an ordered factor with levels "n", 's", "m", "h", "D", "M", "Y". "n" is returned when the date/datetime is complete.

[rossfarrugia]

Sharing summary of discussions over meeting:

• Doug is open to having contributors from our community join the {parttime} effort and help to get this more to a production state - as the use case it was created for never ended up needing it to be at this level, and so there likely will be gaps and areas to improve
• We agreed to focus on the use cases and functionality needed for admiral first - and then we could get a released version to CRAN focusing on these, whilst engaging more of the pharmaverse community to help build out the other pieces over time. Doug showed some really cool features here like how duration calculations could consider the range of possible values accounting for the missingness - I could imagine that being very useful for things like sensitivity analyses (beyond admiral scope!)
• {parttime} would then get added to pharmaverse site, and up to Doug whether we move it over to our GitHub org (the main pro here being the extra credence that comes with being seen as belonging and being supported by pharmaverse community - rather than sometimes packages in individual's orgs could be maybe unfairly perceived to be less well supported/maintained)
• In the short term, the datetime parser piece of the code could be lifted into admiral if we absolutely needed this for next release, with {parttime} being worked up as the planned dependency here so that these pieces would never need to live in admiral long term
• The main gap foreseen is all the odd missing cases we see such as missing month but valid day - like Bill pointed out above

Next steps:

• Stefan to look into the admiral needs (building on what Bill has started here) and share with Doug, and open {parttime} issues for any gaps incl test coverage
• Doug can then consider whether any extra {parttime} issues may be needed to help raise the confidence levels for these features and to be sure CRAN-ready, e.g. more tests, more watertight documentation, etc
• Doug to extend write access to Stefan, Bill, Thomas and Ben, and then our team can jump in with contributing to help plug any gaps. At some stage we can even raise awareness of this in the pharmaverse community slack and sure we might get others interested to help too
• Stefan to make a call on whether we'd need to copy the parser code into admiral as short term solution (in a way that is easy to swap out for {parttime} later) - chat with me/Thomas/Ben if unsure

[bundfussr]

• The most important bit is the update of the parser to allow missings in the middle. There is already an issue for this enhancement in parttime (Handling for missing-in-the-middle datetimes dgkf/parttime#9).
• For now I am using a work-around for the refactoring of the imputation functions. So there is no need to copy code. We can replace it later with a call to the parttime parser.
• Functions for getting and setting components of the partial datetime are already available and the range can be determined by the impute_*() functions.
• I wonder if it would make sense to add a partdate object which considers the date part only.
• I think it would be helpful to get a warning if invalid strings are passed to as.parttime(). As admiral users do not call as.parttime() directly, I am not sure how we can implement it such that the users can identify the variable/observations which caused the issue.

[billdenney]

@bundfussr, thanks for the list. It is helpful. I brought up invalid strings in dgkf/parttime#32, and I made an initial list of some invalid strings for testing in dgkf/parttime#31. Additional examples of valid and invalid strings for testing would help, if you think of them.

[rossfarrugia]

@billdenney just as FYI here's some cases that one of our admiral testers sent to us before that don't work in our current functions: 2003-12-15T-:15:18, 2003-12-15T13:-:19, 2020-07--T00:00:00. I guess the main thing is just testing these different variations of missing middle segments.

[billdenney]

@rossfarrugia , I just made a PR for the test cases that you suggested (and some more variants) in dgkf/parttime#33. The first two are from the SDTMIG and were already in the first PR.

[rossfarrugia]

@bundfussr FYI {parttime} is now on CRAN https://cran.r-project.org/web/packages/parttime/index.html - please can you discuss at the core team meet Weds around whether this will be incorporated as an admiral dependency during this release or next?

[bundfussr]

@rossfarrugia , I have added it to the agenda.

[rossfarrugia]

@billdenney @dgkf we plan to bring the {parttime} dependency in from the release after this one - so during q1 next year as fyi

[billdenney]

That sounds great!

[zdz2101]

@fshanlee Do you think you can still work on this for the Q1 release?

[fshanlee]

@fshanlee Do you think you can still work on this for the Q1 release?

@zdz2101 , I don't think I will have time to work on this. Also, when I last looked at it, I think the {parttime} package implements a type of object as S4. I currently don't know enough about S4 objects and whether it would be simple to incorporate them into {admiral}.

[bms63]

@rossfarrugia @thomas-neitmann @bundfussr Shall we discuss this in our function review/rework meetings? I took off label for Q1-2023 release

[bundfussr]

@rossfarrugia @thomas-neitmann @bundfussr Shall we discuss this in our function review/rework meetings? I took off label for Q1-2023 release

@bms63 , I am not sure. I think we just need somebody who has time to implement it. Or do you questioning that we should use parttime in admiral?

[rossfarrugia]

@bms63 ok for me for this one to move to next release for June if we have too much on already.

[bms63]

Still think we should implement.

[manciniedoardo]

Closing this discussion as there hasn't been much appetite to implement.

[bms63]

necro-time!! @zdz2101

[dgkf]

What timing! I just pushed a new release of parttime (not that it came with new features, just addressing some minor CRAN things).

I think we updated the package last year to include any features that would be necessary for your adoption. If you investigate and find it lacking any critical components, feel free to file an issue! I'm happy to prioritize feature requests to support adoption in admiral.

[bms63]

awesome!! i think we are looking to use it to reduce the complexity of our code. We will be in touch!!

[bms63]

Hey @dgkf!1 parttime package is really nice, but I think admiral is too deep into it for us to adopt unfortunately :(. It seemed like we have some very intricate programming that while parttime might help simplify long-term would end up being a ton of work for us!!

Maybe an admiral2 will be in order one day...sort of how ggplot became ggplot2 :)