Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow polymorphism for dataclasses using type designators #1617

Merged
merged 18 commits into from
Sep 26, 2023

Conversation

cmungall
Copy link
Member

@cmungall cmungall commented Sep 12, 2023

This PR is a continuation of #1257

kervel and others added 4 commits September 12, 2023 10:04
Because the pydantic and the python tests now have similar test cases, i refactored the pydantic test so that they can use common test schema's and data.
This PR incorporates work done in #1257 by @kervel
@cmungall cmungall changed the title feature/poly dataclasses cjm allow polymorphism for dataclasses using type designators Sep 12, 2023
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Patch coverage: 79.47% and project coverage change: -0.07% ⚠️

Comparison is base (8b46560) 80.11% compared to head (5b1c60b) 80.04%.
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1617      +/-   ##
==========================================
- Coverage   80.11%   80.04%   -0.07%     
==========================================
  Files          95       98       +3     
  Lines        9923    10751     +828     
  Branches     2441     2742     +301     
==========================================
+ Hits         7950     8606     +656     
- Misses       1520     1636     +116     
- Partials      453      509      +56     
Files Changed Coverage Δ
linkml/utils/generator.py 87.15% <ø> (-0.03%) ⬇️
linkml/utils/logictools.py 72.24% <72.24%> (ø)
linkml/transformers/logical_model_transformer.py 85.28% <85.28%> (ø)
linkml/generators/jsonschemagen.py 89.71% <85.71%> (-0.32%) ⬇️
linkml/utils/schema_builder.py 81.51% <85.71%> (+0.03%) ⬆️
linkml/generators/pythongen.py 88.42% <87.87%> (-0.04%) ⬇️
linkml/generators/shaclgen.py 85.07% <100.00%> (+0.82%) ⬆️
linkml/transformers/model_transformer.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmungall
Copy link
Member Author

I am not sure why an unrelated test has started failing in windows...

@pkalita-lbl
Copy link
Contributor

The test failures are in fact related to the changes here. These changes introduce SchemaView into PythonGenerator and the failure happens as SchemaView is loading the imports of the schema, kicked off by this line. The failing tests are pre-existing PythonGenerator tests.

It is a bit odd that tests for SchemaView in linkml-runtime or the tests of other generators that use SchemaView haven't tickled this error condition before. I haven't fully characterized what's going on enough to suggest a solution, but in the meantime I don't think we should merge this quite yet.

@cmungall
Copy link
Member Author

cmungall commented Sep 14, 2023

Perhaps the most conservative thing to do is to get the entailed type designators via schemaloader (since pythongen already uses this framework), then we can back out the schemaview dependency.

@kervel do you have any preferences?

We should also try and isolate what is happening with windows and imports as a separate issue/PR

@pkalita-lbl
Copy link
Contributor

That sounds reasonable to me. I have a simple example that demonstrates the issue directly with SchemaView. I'll make a separate issue for it and link it back here.

@pkalita-lbl
Copy link
Contributor

#1627

@cmungall
Copy link
Member Author

On re-reviewing, while it is unusual to mix the two modes, I think on balance it makes sense to use schemaview in this PR. I believe a new runtime release will fix the issue.

cmungall and others added 8 commits September 22, 2023 17:29
* Add source code and test scripts

* Add source code and test scripts

* Fix unsorted imports

* Fix unsorted imports

* Fix unsorted imports

* Fix unsorted imports

* Rename testcase and improve code coverage

* Reformat files
* Add a schema transformer to make logical constraint models.

* add missing

* additional convenience arguments

* lint

* fixed import

* lint

* Formatting

* adding missing file

---------

Co-authored-by: Patrick Kalita <pkalita@lbl.gov>
* Modified tutorial to make age explicitly a string in part1.

Fixes #1612

* fix-typos
* Propagate pattern from type declarations fixes #1371

* adding newline

* adding newline
* [DATALAD RUNCMD] run codespell throughout fixing typo automagically

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

* [DATALAD RUNCMD] fix pytset typo

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "git-sedi pytset pytest",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pythongen should use type_designator to determine specific subtype during instantiation
6 participants