bluesky / scanspec

Specify step and flyscan paths in a serializable, efficient and Pythonic way
Apache License 2.0
8 stars 3 forks source link

Allow vanilla (de)serialization of discriminated unions #132

Closed DiamondJoseph closed 2 months ago

DiamondJoseph commented 2 months ago

Fixes #130, #131

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.25%. Comparing base (f49320c) to head (688265e).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #132 +/- ## ========================================== - Coverage 96.31% 96.25% -0.06% ========================================== Files 9 9 Lines 949 935 -14 ========================================== - Hits 914 900 -14 Misses 35 35 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

DiamondJoseph commented 2 months ago

Current handling requires that subclasses of type annotated with discriminated_union_of_subclasses are Pydantic dataclasses, which I will add to the docstring: this way their schema is generated after Spec has had it schema updated.

Types that want to have their schema updated when a new instance of a class decorated with discriminated_union_of_subclasses must also either be Pydantic dataclasses or BaseModels.

The FastAPI schema for the routes into the service does not update when a new subclass is created, nor do any types that refer to types that refer to a discriminated union.

If Pydantic exposed some way of propagating that a schema that is used in the schema of a model has updated, so the schema of the model must also update, then _TaggedUnion could remove all handling of references and just rebuild_dataclass(self._baseclass)

DiamondJoseph commented 2 months ago

I've made an issue for that last part, but I don't think it will gain much traction as I think creating runtime models often goes against the purpose of validation

https://github.com/pydantic/pydantic/issues/10091