linkml / linkml-runtime

Runtime support for linkml generated models
https://linkml.io/linkml/
Creative Commons Zero v1.0 Universal
25 stars 23 forks source link

Support Pydantic BaseModel in loaders and dumpers #227

Closed kevinschaper closed 1 year ago

kevinschaper commented 1 year ago

Broader objective: https://github.com/linkml/linkml/issues/979

codecov-commenter commented 1 year ago

Codecov Report

Merging #227 (14bb975) into main (33e2013) will increase coverage by 1.87%. The diff coverage is 74.50%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
+ Coverage   63.91%   65.79%   +1.87%     
==========================================
  Files          53       56       +3     
  Lines        6236     7645    +1409     
  Branches     1692     2022     +330     
==========================================
+ Hits         3986     5030    +1044     
- Misses       1769     2019     +250     
- Partials      481      596     +115     
Impacted Files Coverage Δ
linkml_runtime/linkml_model/annotations.py 90.38% <ø> (ø)
linkml_runtime/linkml_model/extensions.py 85.45% <ø> (ø)
linkml_runtime/linkml_model/mappings.py 0.00% <ø> (ø)
linkml_runtime/utils/context_utils.py 68.33% <ø> (ø)
linkml_runtime/dumpers/rdf_dumper.py 65.90% <55.55%> (-5.89%) :arrow_down:
linkml_runtime/loaders/loader_root.py 63.63% <60.00%> (-3.04%) :arrow_down:
linkml_runtime/linkml_model/meta.py 56.57% <72.72%> (+2.63%) :arrow_up:
linkml_runtime/dumpers/json_dumper.py 77.77% <84.61%> (-1.54%) :arrow_down:
linkml_runtime/utils/schemaview.py 87.79% <87.50%> (+4.49%) :arrow_up:
linkml_runtime/dumpers/csv_dumper.py 91.66% <100.00%> (+0.36%) :arrow_up:
... and 8 more

... and 6 files with indirect coverage changes

kevinschaper commented 1 year ago

I see TypeError: Subscripted generics cannot be used with class and instance checks so it looks like I was overly optimistic with:

        if isinstance(results, Union[BaseModel, YAMLRoot]):
            return results
        else:
            raise ValueError(f'Result is not an instance of BaseModel or YAMLRoot: {type(results)}')

and I need to change that to to an OR rather than a union

kevinschaper commented 1 year ago

I'm a little surprised that this is passing with only changes to the method signatures, it sounded like we thought we'd explicitly handle BaseModel differently than YAMLRoot. It could be that we need some tests that try to use BaseModel?

cmungall commented 1 year ago

looks good so far! Next would be to implement the switch

Not surprising this passes, as all you have done is made the signature more inclusive

we should really add mypy typing to the checks we do

kevinschaper commented 1 year ago

Here's where I'm muddling through @hsolbrig - aside from breaking all of the tests, how do these changes look so far?

(I regret changing all of the method signatures first, since it does add a lot of noise to the PR)

kevinschaper commented 1 year ago

currently failing with:

File "pydantic/main.py", line 341, in pydantic.main.BaseModel.__init__
[246](https://github.com/linkml/linkml-runtime/actions/runs/4599915548/jobs/8125893125?pr=227#step:6:247)
pydantic.error_wrappers.ValidationError: 1 validation error for Dataset
[247](https://github.com/linkml/linkml-runtime/actions/runs/4599915548/jobs/8125893125?pr=227#step:6:248)
code_systems -> id
[248](https://github.com/linkml/linkml-runtime/actions/runs/4599915548/jobs/8125893125?pr=227#step:6:249)
  value is not a valid dict (type=type_error.dict)

My memory of this is a little hazy, but I think this is why I created https://github.com/linkml/linkml/issues/1304 - but really, rather than give that job to pydanticgen, I need to normalize before sending to the pydantic class

kevinschaper commented 1 year ago

I'm going to look at moving this forward by using the normalize kitchen sink data file (and maybe corresponding schema?) that pydanticgen is using in the main linkml repo

cmungall commented 1 year ago

I think the data and schema should be in canonical form, but not expanded. E.g. leave prefixes as key-value pairs.