beda-software / fhir-py-types

Convert FHIR StructureDefinition into Python type annotations
BSD 3-Clause "New" or "Revised" License
19 stars 1 forks source link

Support for extensions of primitive datatypes #6

Open axelv opened 1 year ago

axelv commented 1 year ago

I had this validation error when parsing the profiles of the mcode IG:

ERROR:__main__:Error parsing mcode/StructureDefinition-mcode-cancer-related-medication-request.json: 1 validation error for StructureDefinition
snapshot -> element -> 18 -> type -> 1 -> _targetProfile

This has something to do with extensions on primitive data types. In this case an extension of targetProfile

Probably, the library is missing support for extensions on primitive datatypes as specified here: https://build.fhir.org/json.html#primitive

ruscoder commented 2 months ago

I've implemented generation for primitive types here #12

It generates _property: Optional_['Element'] for all primitives including union types (_valueBoolean). And the edge case is also covered for _given: Optional_[List_[Element]] when primitive is repeatable.

ruscoder commented 2 months ago

Unfortunately, pydantic treats all fields starting with an underscore as private and does not validate them.

Also, looks like there are no workarounds, because underscore_attrs_are_private and extra=Extra.forbid mutually don't work.

I propose another solution then:

I'm not sure that it's implementable, but I'm going to try

ruscoder commented 2 months ago

I rethought the suggested approach and it makes the primitive extension model supreme over the primitive types, it's totally wrong because it forces us to use extensions. Also, the suggested approach has issues with type checking, e.g. p.birthDate has type String | str and it can not be used as str without explicit type conversion str(p.birthDate) or accessing p.birthDate.value. The primitive extensions are rare and they just should be supported. It does not make sense to make the development harder in 99% of cases just because of 1% of primitive extension.

I want to develop the initial idea - since the underscore prefix is forbidden to use, let's use the underscore suffix, e.g. birthDate_. I already saw in the code the similar for class_/etc reserved keywords. It can be easily done using pedantic Field with alias

ruscoder commented 2 months ago

Having _ is not enough because it overlaps with reserved keywords, e.g. import_: str and import_: Element. So, let's adhere to fhir.resources python library where it's exposed as __ext suffix. I can not think about anything better, and this is at least well-known for fhir.resources users

ruscoder commented 2 months ago

Just for the history, earlier I had experiments with inheriting primitive data classes from str, like

class String(str):
    def __new__(cls, value, id=None, extension=None):
       ...

class Patient(BaseModel):
    birthDate: String | str

p = Patient(birthDate=String('value', extension=[...]))

Even though it works and type checked, it adds a lot of mystery behind it, e.g. birthDate is str with id/extension attributes. So, I think we should never forget about Python's Zen which states "Explicit is better than implicit." and don't try to do this "magic".