skalarsystems / fhirzeug Goto Github PK

In Observation resource, attribute Subject must receive 0..1 element of type Reference. When using the from_dict function, it must then be a dictionary and not a list. However if a list is passed, no error is raised and weird behavior happens. See example below :

Good example :

resource = r4.from_dict(
    {
        "resourceType": "Observation",
        "status": "final",
        "code": {"coding": [{"system": "test", "code": "test"}]},
        "subject": {"reference": "a reference", "display": "REF"}, # Dictionary -> OK
    }
)
resource.subject
> Reference(id=None, extension=None, reference='a reference', type=None, identifier=None, display='REF')

Wrong type :

r4.from_dict(
    {
        "resourceType": "Observation",
        "status": "final",
        "code": {"coding": [{"system": "test", "code": "test"}]},
        "subject": [{"reference": "a reference", "display": "REF"}], # As a list -> not expected
    }
)
resource.subject
> Reference(id=None, extension=None, reference='display', type=None, identifier=None, display=None)

Expected : a ValidationError must be raised.

From https://www.hl7.org/fhir/json.html:

Property names SHALL be unique. Note: this is not explicitly stated in the original JSON specification,so stated for clarity here

Currently if property name is not unique in JSON, one will be ignored but no error will be thrown.

Hint to solve this issue : https://stackoverflow.com/a/49518779/2750114

From https://www.hl7.org/fhir/datatypes.html#primitive:

The precision of the decimal value has significance:
e.g. 0.010 is regarded as different to 0.01, and the original precision should be preserved
Implementations SHALL handle decimal values in ways that preserve and respect the precision of the value as represented for presentation purposes
Implementations are not required to perform calculations with these numbers differently, though they may choose to do so (i.e. preserve significance)
See implementation comments for XML, JSON and RDF
In object code, implementations that might meet this constraint are GMP implementations or equivalents to Java BigDecimal that implement arbitrary precision, or a combination of a (64 bit) floating point value with a precision field
Note that large and/or highly precise values are extremely rare in medicine. One element where highly precise decimals may be encountered is the Location coordinates. Irrespective of this, the limits documented in XML Schema apply

To be compliant we need to use decimal in Python.

Pydentic seems to support this.

I get a ValidationError : 'OperationOutcome' object is not subscriptable when running the above code. What I want to do is to instanciate the BundleEntry with the outcome instead of a dictionnary.

issue = r4.OperationOutcomeIssue(code="not-found", severity="warning")
outcome = r4.OperationOutcome(issue=[issue])
entry = r4.BundleEntry(resource=outcome)

The exception is thrown due to this code in r4.py :

class BundleEntry(BackboneElement):

    (...)

    resource: typing.Optional["Resource"]

    """ A resource in the bundle.
    Type `Resource` (represented as `dict` in JSON). """
    @pydantic.validator("resource", pre=True,  each_item=True)
    def resource_factory(cls, value):
        return from_dict(value)

BundleEntry expect only to get a dictionnary. What I suggest to do add a line to the code :

    @pydantic.validator("resource", pre=True,  each_item=True)
    def resource_factory(cls, value):
        if isinstance(value, Resource):
             return value
        return from_dict(value)

Change must be done here :

Quantity is generated twice in r4.py file.
This issue is different from issue #16 .

Validation of date time seems to be not working correctly, any number will validate. But not letters.
We need to check why the proposed regex does not work correctly here.

Currently, to export a model, we need to do :

data = resource.dict(by_alias=True)

The idea would be to have by_alias=True as default value since we are never using dict() or json() without this argument.

stack trace:

Traceback (most recent call last):
File "\fhirzeug\fhirzeug\cli.py", line 40, in main
generate(spec)
File "\fhirzeug\fhirzeug\generator.py", line 51, in generate
fhirrenderer.FHIRStructureDefinitionRenderer(spec).render(f_out)
File "\fhirzeug\fhirzeug\fhirrenderer.py", line 97, in render
self.do_render(data, source_path, f_out=f_out)
File "\fhirzeug\fhirzeug\fhirrenderer.py", line 71, in do_render
f_out.write(rendered)
File "C:\Program Files\Python38\lib\encodings\cp1252.py", line 19, in encode
return codecs.charmap_encode(input,self.errors,encoding_table)[0]
UnicodeEncodeError: 'charmap' codec can't encode character '\u2011' in position 638: character maps to

I possible we should try to get more meta information into the generated classes: E.g.:

• Which category and level the resource belongs to.
• The Resource URL

Plan is :

• specify the resource_type attribute as a Literal constant (instead of str)
• in from_dict throw a pydantic.ValidationError if key is not present.

To reproduce issue :

issue.json

{
  "resourceType": "HealthcareService",
  "id": "example",
  "contained": [
    {
      "resourceType": "",
      "id": "DenBurg",
      "description": "Großraum Denburg",
      "mode": "instance",
      "physicalType": {
        "coding": [
          {
            "code": "area",
            "display": "Area"
          }
        ]
      }
    }
  ]
}

script.py

import json
from pathlib import Path
from pydantic_fhir import r4

data = json.load(Path("issue.json").open())
r4.from_dict(data)

Stacktrace :

Traceback (most recent call last):
  File "script.py", line 8, in <module>
    r4.from_dict(data)
  File "/home/projects/skalar/fhirzeug/pydantic-fhir/pydantic_fhir/r4.py", line 54302, in from_dict
    return RESOURCE_TYPE_MAP[dict_["resourceType"]](**dict_)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/main.py", line 344, in __init__
    values, fields_set, validation_error = validate_model(__pydantic_self__.__class__, data)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/main.py", line 900, in validate_model
    v_, errors_ = field.validate(value, values, loc=field.alias, cls=cls_)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/fields.py", line 575, in validate
    v, errors = self._validate_sequence_like(v, values, loc, cls)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/fields.py", line 606, in _validate_sequence_like
    r, ee = self._validate_singleton(v_, values, v_loc, cls)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/fields.py", line 712, in _validate_singleton
    value, error = field.validate(v, values, loc=loc, cls=cls)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/fields.py", line 564, in validate
    v, errors = self._validate_singleton(v, values, loc, cls)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/fields.py", line 719, in _validate_singleton
    return self._apply_validators(v, values, loc, cls, self.validators)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/fields.py", line 726, in _apply_validators
    v = validator(cls, v, values, self, self.model_config)
  File "/home/.cache/pypoetry/virtualenvs/pydantic-fhir-XG3S17Ik-py3.6/lib/python3.6/site-packages/pydantic/class_validators.py", line 278, in <lambda>
    return lambda cls, v, values, field, config: validator(cls, v)
  File "/home/projects/skalar/fhirzeug/pydantic-fhir/pydantic_fhir/r4.py", line 8519, in resource_factory
    return from_dict(value)
  File "/home/projects/skalar/fhirzeug/pydantic-fhir/pydantic_fhir/r4.py", line 54302, in from_dict
    return RESOURCE_TYPE_MAP[dict_["resourceType"]](**dict_)
KeyError: 'resourceType'

Now the fhir spec is mapped 1:1, to be more pythonic snake case should be supported.

ATM the validation of empty object is only working in dict form.

Example:

 RootModel(
        field_a="a", list_items=[ListItem(), ListItem(), ListItem()]
    ).dict() == {"field_a": "a"}

does not raise an validation error. But it actually should.

RootModel(**{"field_a": "a", "list_items": [{}, {}, {}]})

Does.

The problem is in resource header strip_empty_items since it only takes care about the dict representation of the model, which is not executed in case of the construction via Objects. A similar functionality needs to be implemented on a Pydantic object level without the pre parameter in the validator. This should then cover the json variant as well.

At the moment, black and mypy are added in dev packages but not used in the github workflow.
Should we automatically do the check ? It will require code development since it doesn't pass the check locally.

i'm using some FHIR profiles which have StructuredDefinitions (https://simplifier.net/ILCore/ILCorePatient/~json for example)
i wonder if there's a way to use your generator to generate the classes for these profiles...

i'd love your advice (and maybe even help in the development)

If I understand well, FHIRUnitTest generates the unit test for the classes. Which means that it is now unused since we use pydantic package for validation. Therefore, I suggest to remove it completely. That would give a boost to code coverage since no tests are done on this part.

According to https://www.hl7.org/fhir/json.html:

Given the way extensions work, applications reading JSON resources will never encounter unknown properties. However, once an application starts trading with other applications that conform to later versions of this specification, unknown properties may be encountered. Applications MAY choose to ignore unknown properties in order to foster forwards compatibility in this regard, but may also choose not to.

Also, the JSON spec version of the schema defines "additionalProperties": false for all object types.

Right now, unknown fields are ignored, but maybe a better idea is to throw ValidationError if they appear. It could be easily done by adding extra = 'forbid' to the Config class in FHIRAbstractBase.

Instead the enum repr is taken.

@auerj already fixed that by using a str mixin.

Issue first addressed in #47 . Ticket is splitted since two issues were initially reported.

Issue :
0.00 turns into 0.0 for float fields

To be more precise, float fields are at the moment handled using decimal.Decimal object to keep best possible precision. Problem arises when we use pydantic_fhir along FastAPI.

If a JSON-primitive field is required, it is possible to provide only an extension instead. At the moment, this behavior is NOT implemented (a required primitive field must always be provided with a primitive value).

FHIR spec : https://www.hl7.org/fhir/extensibility.html#Special-Case

Serialized objects do contain empty fields, this is not wished by fhir: https://www.hl7.org/fhir/datatypes.html#representations

Suggestion by @trojanowski :

def _without_empty_items(obj: typing.Any):
    """
    Clean empty items: https://www.hl7.org/fhir/datatypes.html#representations
    TODO: add support for extensions: https://www.hl7.org/fhir/json.html#null
    """
    if isinstance(obj, Mapping):
        cleaned = {}
        for key, value in obj.items():
            cleaned_value = _without_empty_items(value)
            if cleaned_value is not None:
                cleaned[key] = cleaned_value
​
        if cleaned:
            return cleaned
        return None
​
    if isinstance(obj, str):
        if not obj:
            return None
        return obj
​
    if isinstance(obj, (list, tuple)):
        cleaned = [_without_empty_items(item) for item in obj]
        if any((item is not None for item in cleaned)):
            return cleaned
        return None
​
    return obj
​
​
class FHIRAbstractBase(pydantic.BaseModel):
    """Abstract base class for all FHIR elements.
    """
​
    def dict(self, *args, **kwargs):
        serialized = super().dict(*args, **kwargs)
        return _without_empty_items(serialized)

ImportError while importing test module '/home/julian/src/pydantic-fhir/tests/test_import_examples.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../.cache/pypoetry/virtualenvs/pydantic-fhir-o_sDi3hj-py3.8/lib/python3.8/site-packages/_pytest/python.py:511: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
../../.cache/pypoetry/virtualenvs/pydantic-fhir-o_sDi3hj-py3.8/lib/python3.8/site-packages/py/_path/local.py:704: in pyimport
    __import__(modname)
../../.cache/pypoetry/virtualenvs/pydantic-fhir-o_sDi3hj-py3.8/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:152: in exec_module
    exec(co, module.__dict__)
tests/test_import_examples.py:7: in <module>
    from pydantic_fhir import r4
pydantic_fhir/r4.py:3: in <module>
    import stringcase
E   ModuleNotFoundError: No module named 'stringcase'

When generating the code the generated pyproject.toml is missing the stringcase dependency. This is also not recognized by the ci.

Fix:

• Break it on the CI
• Add it to the dep file

According to FHIR specs, duration_unit attribute must be one of "s | min | h | d | wk | mo | a" ( https://www.hl7.org/fhir/datatypes.html#Timing ).
However, in the current implementation duration_unit is encoded as a FHIRCode which is not suited for enums.

Minimal example :

import r4

repeat_data = {
    "duration_unit": "not_a_predefined_unit",
}
repeat = r4.TimingRepeat(**repeat_data) # Must raise a ValidationError

Additional information about fhir codes : https://www.hl7.org/fhir/datatypes.html#code

EDIT :
The problem is not about DocEnum validation. This code has the expected behavior (raises a ValidationError).

import r4

account_data = {
    "status": "not_a_predefined_status",
}
account = r4.Account(**account_data) # Raise an error because `AccountStatus` is a `DocEnum` object.

https://www.hl7.org/fhir/search.html

Stringcase allows string formatting between lowercase, uppercase, camelcase, snakecase,...
At the moment, some formatting are done manually and might introduce error.

For instance, I spotted this code in fhirspec.py :

 # CamelCase or just plain
if self.settings.camelcase_classes:
     return classname[:1].upper() + classname[1:]
return classname

Maybe we could generalize the use of stringcase package to avoid any issue.

Numbers are truncated in several ways:

• 0.00 turns into 0.0 for float fields
• If a field is defined as an integer it truncates floats: 1.23 -> 1. This is information loss and can not be tolerated in FHIR.

https://www.hl7.org/fhir/json.html#primitive

Update black version to 20.* in order to take advantage of the "magic comma feature" (https://black.readthedocs.io/en/stable/the_black_code_style.html#the-magic-trailing-comma)

Add isort everywhere to sort imports correctly ? https://pypi.org/project/isort/

From specs (https://www.hl7.org/fhir/extensibility.html#Extension) :

An extension SHALL have either a value (i.e. a value[x] element) or sub-extensions, but not both. If present, the value[x] element SHALL have content (value attribute or other elements)

This is not currently validated in the generated Extension Element.

Deserialzing

{
                "resourceType": "ClinicalImpression",
                "status": "completed",
                "subject": {"reference": ""},
}

Leads to a internal representation of:

resource_type='ClinicalImpression' id=None meta=None implicit_rules=None language=None text=None contained=None extension=None modifier_extension=None identifier=None status=<EventStatus.completed: 'completed'> status_reason=None code=None description=None subject=Reference(id=None, extension=None, reference=None, type=None, identifier=None, display=None) encounter=None effective_date_time=None effective_period=None date=None assessor=None previous=None problem=None investigation=None protocol=None summary=None finding=None prognosis_codeable_concept=None prognosis_reference=None supporting_info=None note=None

Which then leads to a serialization of, which is an invalid ClinicalImpression since subject has a carnality of 1...1:

{
                "resourceType": "ClinicalImpression",
                "status": "completed"
}

This behavior is half correct. The subject should actually be stripped before de-serialization, which then would lead to a ValidationError, because subject is a required field.

Context:

Reference element has a reference attribute of type string. It is currently validated by pydantic_fhir as a string.
The idea is to add a custom validator to also validate the consistency of the URL.

From https://www.hl7.org/fhir/references-definitions.html#Reference.reference :

Reference.reference : A reference to a location at which the other resource is found. The reference may be a relative reference, in which case it is relative to the service base URL, or an absolute URL that resolves to the location where the resource is found. The reference may be version specific or not. If the reference is not to a FHIR RESTful server, then it should be assumed to be version specific. Internal fragment references (start with '#') refer to contained resources.

Using absolute URLs provides a stable scalable approach suitable for a cloud/web context, while using relative/logical references provides a flexible approach suitable for use when trading across closed eco-system boundaries. Absolute URLs do not need to point to a FHIR RESTful server, though this is the preferred approach. If the URL conforms to the structure "/[type]/[id]" then it should be assumed that the reference is to a FHIR RESTful server.

Also, from https://www.hl7.org/fhir/references-definitions.html#Reference.type:

Reference.type : The expected type of the target of the reference. If both Reference.type and Reference.reference are populated and Reference.reference is a FHIR URL, both SHALL be consistent.

According to this, two conditions must be validated:

• If Reference.reference field (relative or absolute URL) use the /type/id pattern and Reference.type is populated, they must match.
• If Reference.reference field doesn't use the /type/id pattern then it is considered to be an absolute URL and must be validated with pydantic URL validator.

Additional material:

• a regex to parse a literal reference to a FHIR REST server : https://www.hl7.org/fhir/references.html#literal
• defintion of a service base url : https://www.hl7.org/fhir/http.html#root

Idea : find a way to include metainformation (namely URLs) to the generated DocEnum classes (Valuesets). Purpose is to be able to use it as a Python attribute.

This issue is related but not duplicate from issue #21 (and PR #33 ). I advice to read them before starting this issue.

ATM we have docstrings for enums, but they are not quite nice.

Some investigation is probalby needed how to do that in the correct way, so the schema generation of pydantic also recognizes the docstrings. What I found is:

https://stackoverflow.com/questions/52062831/how-do-i-properly-document-python-enum-elements

@julian-r started to implement a generator.yaml config file to save settings. Goal is to expand its use.
It might also be the right time to definitely remove the (untested and deprecated) python generator.