feat: Support for Device Composite injection #1231
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1231 +/- ##
==========================================
+ Coverage 94.66% 94.67% +0.01%
==========================================
Files 41 41
Lines 2604 2611 +7
==========================================
+ Hits 2465 2472 +7
Misses 139 139 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/blueapi/core/context.py
Outdated
| def _inject_composite(self, composite_class: type[C]) -> C: | ||
| devices = { | ||
| field: self.find_device(info.default) | ||
| if info.annotation is not None | ||
| and is_bluesky_type(info.annotation) | ||
| and isinstance(info.default, str) | ||
| else info.default | ||
| for field, info in composite_class.model_fields.items() | ||
| } | ||
| return composite_class(**devices) |
There was a problem hiding this comment.
This handling requires that all devices are registered before any plan that uses a Composite BaseModel, which previously was not a requirement. We can either document that, or possibly we could recursively treat the BaseModel by passing it back into _type_spec_for_function? Then the Device classes in the Composite get replaced by References that know how to be deserialised from the context, and devices can be registered after plans?
There was a problem hiding this comment.
I've updated it such that the factory it returns is a genuine factory rather than a factory wrapping an instance.
I don't think there's a requirement that the device composites support nesting, at least there certainly isn't for Hyperion
| default_factory = ( | ||
| self._composite_factory(arg_type) | ||
| if isclass(arg_type) | ||
| and issubclass(arg_type, BaseModel) |
There was a problem hiding this comment.
I think the only weird edge case I can think of here is if someone has made a Device that extends BaseModel, but I think that breaks the child handling.
The use of a default factory here is much simpler than what I was thinking, I really like this.
1313d57 to
944be07
Compare
This modifies the previous PR #1231 and adds the following: * Simplifies the creation of composites, the composite no longer needs to be an instance of `BaseModel` and instead can be a dataclass * The composite does not need to specify a default and instead determines the device to inject from the attribute name
This introduces support for composite devices in blueapi plan signatures.
Composite devices must extend from pydantic
BaseModel, and the devices contained in them will be obtained from theBlueskyContext