search

nazrulworld / fhir.resources

FHIR Resources https://www.hl7.org/fhir/resourcelist.html
https://pypi.org/project/fhir.resources/
Other
372 stars 104 forks source link

Mypy errors accessing resources fields #60

Open ItayGoren opened 3 years ago

ItayGoren commented 3 years ago

Im using python 3.8 and trying to upgrade from version 6.0.0b5 to 6.1.0. The code is

from fhir.resources.codeableconcept import CodeableConcept
from fhir.resources.condition import Condition

Condition(subject={}, code=CodeableConcept()).code.coding

When using version 6.0.0b5 and running mypy --ignore-missing-imports a.py I get: Success: no issues found in 1 source file

Using version 6.1.0 and running the same function I get:

a.py:4: error: "CodeableConceptType" has no attribute "coding"
Found 1 error in 1 file (checked 1 source file)

Looking at the classes, we can see that the annotations are using FooType, for example CodeableConceptType and not CodeableConcept which makes mypy not being able to parse the library. If I understand correctly, the rational to use FooType classes is otherwise we would have circle imports.

How can we make mypy parse this library? Or, what changed between 6.0.0b5 to 6.1.0 that can affect it? Were type hints added to the library? If so, I think its wrong to do so :)

nazrulworld commented 3 years ago

Related issue https://github.com/nazrulworld/fhir.resources/issues/58

I don't think we did some changes (from 6.0.0b5 to 6.1.0) related to Mypy/linting.

Are you using the same mypy version or upgraded?

ItayGoren commented 3 years ago

same one. the problem is that we didnt have typing before - it didnt find it and because i use --ignore-missing-imports i had no problems

ItayGoren commented 3 years ago

I continued to think about this problem and correct me if im wrong, but the ClassNameType is used because without it there is a circular imports flow. This circle occur because of Extensions which holds primitives and each primitive holds Extension. Is that the case? If so, putting all of them in the same file would solve it. What do you think about it? Another solution, is specifying the annotations in Extension using strings and not classes and do that only there. What do you think? @nazrulworld

nazrulworld commented 3 years ago

you are right that primarly ClassNameType is used to solve FHIR's two-way relationship which introducing circular dependency. but this typing is also used as Pydantic field validator!

putting all of them in the same file would solve it. What do you think about it? I don't think that will solve the problem beside the problem, not just inside Extension, other places as well. For example see relationship between Element <-> Extension class.

Another solution, is specifying the annotations in Extension using strings and not classes and do that only there. That could be a good idea, let's try!