Duplicate Quantity resource class

[Wauplin]

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

[Wauplin]

pydantic_fhir/r4.py:27905:1: F811 redefinition of unused 'Quantity' from line 27335
pydantic_fhir/r4.py:28016:1: F811 redefinition of unused 'Age' from line 27960
pydantic_fhir/r4.py:28029:1: F811 redefinition of unused 'Count' from line 27973
pydantic_fhir/r4.py:28046:1: F811 redefinition of unused 'Distance' from line 27990
pydantic_fhir/r4.py:28059:1: F811 redefinition of unused 'Duration' from line 28003

Age, Count, Distance and Duration are datatypes inherited from Quantity. I believe that solving the problem for Quantity will solve everything.

[Wauplin]

Quantity class seemed to be rendered twice due to a conflict with the MoneyQuantity resource.

MoneyQuantity is a profile of Quantity that implements a new constraint. Documentation : http://hl7.org/fhir/datatypes.html#MoneyVariations . Especially there is this sentence : Note that the profile is different from the other specializations because it is not a type, just rules applied where the Quantity type is used to represent Money amounts. I am not sure but maybe it just means that we shouldn't care about creating the MoneyQuantity class at all ? This might be related to this topic : https://github.com/FirelyTeam/fhir-net-api/issues/999#issuecomment-498581086 .

[Wauplin]

@julian-r : I might be wrong about investigating the Quantity and MoneyQuantity difference. I start to think that the code implementation is good and that MoneyQuantity must not be a separate generated class. The only problem is the fact that we render twice the same class.

This is done by the following code in fhirrenderer.py :

        # <--- Wauplin's note : Loop looks for every class to generate
        for profile in self.spec.writable_profiles(): 
            classes = profile.writable_classes()
            for cl in classes:
                derive_graph.setdefault(cl.superclass_name, []).append(cl)

        classes = [] # <----- Wauplin's note : classes that will actually be rendered
        work_stack = [
            "FHIRAbstractBase",
            "FHIRAbstractResource",
        ]

        # <---- Wauplin's note : Loop populates an ordered list of classes to generate (given the dependencies)
        while work_stack:
            current = work_stack.pop()
            for elm in derive_graph.get(current, []):
                work_stack.append(elm.name)
                classes.append(elm)

A quick workaround is to check in the second loop if elm is already in classes before appending it.

                if elm not in classes:
                    classes.append(elm)

I tried it and it works fine :

• no duplicated classes in generated code
• all tests have passed (both fhirzeug and generated tests)

@julian-r : my biggest concern about this workaroung is that I might be silencing a major error. My feeling is that it's not since we are rendering exactly the same FHIRClass python object but I would like your opinion about it.

[Wauplin]

NB : If an element/class is defined twice with the same name is the JSONs (like Quantity / MoneyQuantity), only the first definition leads to the creation of a FHIRClass object. Next times the class is defined, the same FHIRClass object is returned :

class FHIRClass:
    """ An element/resource that should become its own class.
    """

    known: Dict[str, "FHIRClass"] = {}

    @classmethod
    def for_element(cls, element):
        """ Returns an existing class or creates one for the given element.
        Returns a tuple with the class and a bool indicating creation.
        """
        assert element.represents_class
        class_name = element.name_if_class
        if class_name in cls.known:
            return cls.known[class_name], False # <------ next times
        klass = cls(element, class_name)
        cls.known[class_name] = klass
        return klass, True # <------ first time
        (...)

Quantity profile definition : https://www.hl7.org/fhir/quantity.profile.json.html MoneyQuantity profile definition : https://www.hl7.org/fhir/moneyquantity.profile.json.html (see differential key)

[Wauplin]

EDIT : I just realized that SimpleQuantity profile is currently skipped in the generation (see fhirspec.py l.20) :

# TODO: check
# allow to skip some profiles by matching against their url (used while WiP)
skip_because_unsupported = [
    r"SimpleQuantity",
]

SimpleQuantity and MoneyQuantity might have similar issues. This piece of code comes from the initial fork.

[julian-r]

I think learning from the firly guys and going down the same path makes a lot of sense :+1:

[Wauplin]

@julian-r : I might be wrong about investigating the Quantity and MoneyQuantity difference. I start to think that the code implementation is good and that MoneyQuantity must not be a separate generated class. The only problem is the fact that we render twice the same class.

This is done by the following code in fhirrenderer.py :

        # <--- Wauplin's note : Loop looks for every class to generate
        for profile in self.spec.writable_profiles(): 
            classes = profile.writable_classes()
            for cl in classes:
                derive_graph.setdefault(cl.superclass_name, []).append(cl)

        classes = [] # <----- Wauplin's note : classes that will actually be rendered
        work_stack = [
            "FHIRAbstractBase",
            "FHIRAbstractResource",
        ]

        # <---- Wauplin's note : Loop populates an ordered list of classes to generate (given the dependencies)
        while work_stack:
            current = work_stack.pop()
            for elm in derive_graph.get(current, []):
                work_stack.append(elm.name)
                classes.append(elm)

A quick workaround is to check in the second loop if elm is already in classes before appending it.

                if elm not in classes:
                    classes.append(elm)

I tried it and it works fine :

• no duplicated classes in generated code
• all tests have passed (both fhirzeug and generated tests)

@julian-r : my biggest concern about this workaroung is that I might be silencing a major error. My feeling is that it's not since we are rendering exactly the same FHIRClass python object but I would like your opinion about it.

-> Ok so I implemented this solution in the linked PR.