feat: thermistor assembly and implant#1866
Conversation
…om/AllenNeuralDynamics/aind-data-schema into 1585-thermistor-device-and-implant
…om/AllenNeuralDynamics/aind-data-schema into 1585-thermistor-device-and-implant
| class DeviceImplant(ProtocolMixin, DevicePosition, DataModel): | ||
| """Description of a generic device implant procedure""" | ||
|
|
||
| implanted_device: str = Field( |
There was a problem hiding this comment.
won't we always need the implanted_device to take a Device object? So a generic device should take Device rather than be a string.
There was a problem hiding this comment.
And then is it possible to make it a discriminated list that can be Device or ThermistorAssembly and if we have other implanted devices that can't just be a device we could add them in the future?
There was a problem hiding this comment.
There's some reason that I made it a str... but now I don't remember what it was. I simplified to just DeviceImplant with implanted_device: Device. Hopefully that doesn't break anything
saskiad
left a comment
There was a problem hiding this comment.
first, I don't think we need the connector for the assembly. But I'm willing to be persuaded.
second, I think we can just use the generic DeviceImplant with a discriminated list
…om/AllenNeuralDynamics/aind-data-schema into 1585-thermistor-device-and-implant
saskiad
left a comment
There was a problem hiding this comment.
I think this is good. I still feel like ProbeConfig and DevicePosition are so similar that there could be a way to combine them, but I think it's not obvious...
Just clean up the stuff in the example so people don't copy that.
Adds a new DeviceImplant to support thermistor wire implants, and adds an example showing how to use it. Molex is not an available manufacturer, we'll have to add it separately (we can do that before merging, if we want).