-
Notifications
You must be signed in to change notification settings - Fork 89
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
[pydanticgen] Embed extra metadata in modules, classes, and fields #2036
Conversation
…pliance tests to not close expected python annotations. make arraytypes a string enum
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2036 +/- ##
==========================================
- Coverage 80.67% 80.62% -0.05%
==========================================
Files 107 108 +1
Lines 11943 12011 +68
Branches 3415 3433 +18
==========================================
+ Hits 9635 9684 +49
- Misses 1743 1757 +14
- Partials 565 570 +5 ☔ View full report in Codecov by Sentry. |
alright, this is ready to check out - again i have held off adding tests for this until i get a better idea if this is the kind of thing we want, but if i get a nod i'll go ahead and add them |
How about adding some command line options to control
|
this is already in there, but not as a cli option. will add!
want this in this PR or in one after we make the runtime import? :) |
let's save the inlined vs runtime as a separate PR. Incremental is good! (as is preserving default behavior) |
Hi @sneakers-the-rat and @cmungall, |
After this PR, all metadata will be (optionally) included. Until then, all fields that are in the template models have some representation in the generated pydantic models https://linkml.io/linkml/generators/pydantic.html#templates |
Thanks, @sneakers-the-rat, Looking forward to the merge - do you have a rough time estimate, when it is scheduled ? One last question: will URIs (like slot_uri) also be transferred to the pydandic output ? |
Yes, all metadata. It will be configurable depending on if you want literally every field in the source schema, only those fields not represented by the template models, or no metadata. See an example in the first post in this issue.
Behavior of the template classes is already documented. This PR will also be documented after we reach a final form for it.
Im AFK until next week. Sometime after that. Still need to write tests and docs and decide implementation details
All metadata |
Thanks a lot, @sneakers-the-rat, |
Hi @sneakers-the-rat , |
@sneakers-the-rat - thanks a lot for working on this! I was trying to test it, but I'm not completely sure how I can use |
Aha, what I think we'll do is expose that as a param instead of having to fiddle with the template classes, since that's likely to come up a lot. do you mean to override the default |
this was not quite done but i can follow on in another PR.
|
Fix: #2005
Related to:
I feel like this comes up a lot. with the new template system it was pretty easy to implement.
this PR adds all metadata that isn't explicitly excluded - either from being already represented by the template model, or by being present in their
meta_exclude
classvars - to alinkml_meta
attribute in modules, classes, and fields.opening this as a draft because i figure there is plenty of disagreement to be had about where to put them, what should be excluded, etc. but this general framework works.
Currently using
json_schema_extra
in linkml 2 to store it, because themetadata
field isn't really for this, but we can also talk about where that should go - bonus of that is we get all the extra metadata in pydantic's generated json schema for free :)but anyway here's an overview using
personinfo.yaml
as a sample:Adds a
LinkMLMeta
class that is basically a subclass of dict:then schema metadata looks like this:
class metadata is like this:
attribute metadata is like this:
Access to metadata is simple and uniform, even if the attribute version is a little verbose
we can do more sophisticated transformations of the embedded values like casting prefixes to a specific prefix class, filtering
"alias"
if it is identical to the name of the attribute, etc. too. That'll be easier once PRs like #2019 get merged and the rendering logic for each type of object is more separated - i'd prefer to wait on that so i can clean this up, i don't really like adding to the junk heap i made at the bottom ofserialize()
lol, but figured i was worth getting a draft on the books so we have something to point to when this comes up, as it often does.no tests yet, wanted to wait for feedback before trying to finish it