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

What's the best way to compare periods? #17

Closed
mmabey opened this issue Jun 30, 2020 · 5 comments
Closed

What's the best way to compare periods? #17

mmabey opened this issue Jun 30, 2020 · 5 comments
Labels
question Further information is requested

Comments

@mmabey
Copy link
Contributor

mmabey commented Jun 30, 2020

  • fhir.resources version: 5b98990
  • Python version: 3.8.3 🎉
  • Operating System: Mac OS 10.15.5

I've been taking a look at the pydantic refactored code, and I think it's going to be a great thing for the project. As I've looked at how to adapt my code to the changes, one thing has come up that I can't quite find a good answer for.

In the project I'm working on, there are lots of instances where I need to be able to compare one Period with another. For example, I need to know which period is more recent than another. This gets really tricky because FHIR's dateTime values have such a wide range of acceptable granularity. For example, I might need to decide if a period with a starting year, month, and day is more recent than a period with an ending year and month (but no day).

The solution I came up with previously was to not depend on the Python datetime objects offered by the previous (<=5.0.1) versions of fhir.resources, but instead parse the FHIR start and end dateTime values myself and slightly differently, with start dates filling any missing values for month, day, hour, minute, second, and microsecond with the lowest (earliest) value (e.g., month=1, hour=0, etc). Likewise, I parsed end dateTime values by filling any missing values with the highest (latest) value possible (e.g., month=12, minute=59).

The above solution depended on using the origval property of the fhir.resources.period.Period object, which I used as input to the custom parsers I just described. And this is where the problem arises: The new pydantic models do not retain the original string extracted from the FHIR-formatted JSON.

I've tried to do some research into possible ways to address this, but due to my lack of familiarity with the ins and outs of pydantic, I've probably missed some viable solutions. Anyway, here are some ideas:

  1. Alter the pydantic models for Date, DateTime, Instant, and Time to make the original string value available (the ones defined in fhir/resources/{DSTU2,STU3,}/fhirtypes.py. I list this first because I think it will be the least labor-intensive, assuming there's a way to actually accomplish this with pydantic. From what I understand, these model types would need to use generic classes

    • Pros:
      • Provides enough flexibility so that custom parsers (like my own) can do what is necessary for the specific circumstances.
    • Cons:
      • Breaks some of the purity of the pydantic models/types of the library.
      • Doesn't provide a generic solution, meaning projects must each develop their own approach to solving the same problem of comparing date, dateTime, and time values.
      • May require the classes to start returning instances of themselves from the validate methods instead of the classes from the datetime library. This is only a half-con though since I think the pydantic docs recommend doing it that way anyway.
  2. Create a custom class for Periods that allow for fuzzy data (according to the FHIR spec) while also providing operators that allow for comparison in a way that makes sense (I drew up a few examples of this but eventually decided they were confusing).

    • Pros:
      • Provides a "batteries included" kind of way to compare Period elements without library users having to implement anything themselves.
    • Cons:
      • Implementation could get a little complicated.

That's all I've got so far. I'm hoping that by discussing this we can find something that will be useful to many of the library's users.

@nazrulworld
Copy link
Owner

@mmabey first of all thank you much for your help to improve the strength of this package.

However, I can explain about Date, dateTime of FHIR.

  1. as we know about flexibility of FHIR date format (some of incompatible with python date, datetime), to keep that in mind, fhirtypes.Date and DateTime returns string value if parts of date provided, for example only year and/or month. [we don't want loose original information in general], even DateTime type may return Date object if only year-month-day is provided.

What about business requirements like yours?

This library uses pydantic's validator strength to allow custom root validator for each model. I cannot make full documentation yet. But here example code

from fhir.resources.period import Period

def my_pre_period_validator(cls, values):
    """ """
    raw_start = values["start"]
    raw_end = values["end"]
    # do check raw values, if need add missing part of
    # date. even possible to make date object
    values["start"] = raw_start # modified
    values["end"] = raw_end # modified

Period.add_root_validator(my_pre_period_validator, pre=True)

this function should be called my_pre_period_validator before constructing Period object, here you will have access to raw value.

@nazrulworld nazrulworld added the question Further information is requested label Jun 30, 2020
@mmabey
Copy link
Contributor Author

mmabey commented Jun 30, 2020

This is a very interesting feature of pydantic, thanks for sharing!

If I'm understanding your example correctly, the modified Period class would only be accessible to code that imported Period from the Python file where this modification takes place, for example (assuming your example is stored in my_fhir_shim.py):

from my_fhir_shim import Period

Is this ☝️ correct? If so, I'm not sure it helps me. For all of the FHIR objects I construct, I don't know beforehand what resource type I'm creating, so I'm making heavy use of the FHIRElementFactory/construct_fhir_element feature of this library.

In any case, I'm not creating Period objects directly, they're all being created as a sub-element (sometimes a sub-sub-sub-element) of something else. For example, a Claim resource has an Identifier which contains a Period. I'll make some local tests to see if it's possible to use a root validator, but if it isn't possible, do you have any other suggestions?

@nazrulworld
Copy link
Owner

I think you are almost got it, I am trying to minimize some misunderstanding. I am taking your example, the file name is my_fhir_shim .py.

  1. once this my_fhir_shim module is imported anywhere at runtime, for example in your package root __init__.py import my_fhir_shim That's all!
  2. you don't need to from my_fhir_shim import Period and should not worry about whether you are constructing Period manually or automatically by pedantic. All should work as expected, your function will be called before period instance creation is done.
  3. import Period from the standard place if you are constructing manually.
  4. Keep in mind this should bePeriod.add_root_validator(my_pre_period_validator, pre=True) in one place across your project per function, which means the same function should not add multiple time.

@mmabey
Copy link
Contributor Author

mmabey commented Jul 1, 2020

That makes so much more sense. Thank you for clarifying! It looks like it will be a while before I'm able to make another attempt at using a root validator in my project. I'll reopen if I'm unable to figure it out. Thanks again!

@mmabey mmabey closed this as completed Jul 1, 2020
@mmabey
Copy link
Contributor Author

mmabey commented Aug 11, 2020

For anyone that finds this thread because they want to know how to add a root validator to a pydantic model without touching the model itself, there's one critical thing that was missing from @nazrulworld's example above. The validator function must return the values. Here's approximately what I'm doing in my validator:

from typing import Dict

from fhir.resources.DSTU2.period import Period

from .parse_helpers import parse_end_time, parse_start_time


def normalize_period_dates(cls, values: Dict):
    if not values:
        return values
    raw_start = values.get("start")
    raw_end = values.get("end")
    if raw_start:
        values["start"] = parse_start_time(raw_start)
    if raw_end:
        values["end"] = parse_end_time(raw_end)
    return values


class Shims:
    _done = None

    @classmethod
    def setup_shims(cls):
        if Shims._done:
            return
        # Add all root validators here
        Period.add_root_validator(normalize_period_dates, pre=True)

        Shims._done = True


Shims.setup_shims()

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

No branches or pull requests

2 participants