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

need help with abstract groups #15

Closed
tlambert03 opened this issue Jul 21, 2020 · 6 comments
Closed

need help with abstract groups #15

tlambert03 opened this issue Jul 21, 2020 · 6 comments

Comments

@tlambert03
Copy link
Owner

Looking at one of the failing test files (spim.xml) I see that we're getting the error:

pydantic.error_wrappers.ValidationError: 2 validation errors for OME
instrument -> 0
  __init__() got an unexpected keyword argument 'laser' (type=type_error)

spim.ome.xml has an <Instrument> with a child <Laser> (which I gather is one of the concrete implementations of a LightSourceGroup?) ... whereas the generated model is looking for the literal key light_source_group...

@jmuhlich, Code generation aside for the moment, what should the behavior be? should it be that, any children of Instrument that are concrete subtypes of LightSource should be added to the light_source_group list during __init__? If I have that correct, I can probably make it happen somehow... but curious to hear your thoughts

@tlambert03
Copy link
Owner Author

same deal with ShapesGroups probably...

@jmuhlich
Copy link
Collaborator

Yes, as a user of an Instrument class I'd expect that inst.light_source_group or inst.light_sources is a list of instances of the LightSource subclasses. Working on that now.

@jmuhlich
Copy link
Collaborator

One further wrinkle is xmlschema's to_dict is not wrapping e.g. the content under laser, arc, etc. in a list if there's only one entry, probably because it's working off the Laser definition in the schema and not Instrument/LightSourceGroup which has maxOccurs="unbounded". I wonder if we can hack around this in our schema.MyConverter class.

@tlambert03
Copy link
Owner Author

Since the dict is just a temporary intermediate... I think we could just as easily handle that during dataclass instantiation, see #13 (comment) ...

@jmuhlich
Copy link
Collaborator

Thinking about it, this is actually something that pydantic should be able to help us out with. Looks like it can actually handle Unions to some degree, but the members of the Union all have to be sort of "disjoint" in what their validators would accept, as pydantic just tries to decode with all of the members in sequence until one validates. This would probably work for us:
https://pydantic-docs.helpmanual.io/usage/types/#unions

There is also some ongoing work on "disciminated unions" where you declare a specific field in your subclasses that is assigned a unique value in each one, but it's not ready yet:
pydantic/pydantic#619

@jmuhlich
Copy link
Collaborator

jmuhlich commented Aug 3, 2020

Addressed with overrides in #17 and #21.

@jmuhlich jmuhlich closed this as completed Aug 3, 2020
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

No branches or pull requests

2 participants