spc-group / guarneri

Other
0 stars 0 forks source link

Allow device classes to be specified as direct imports #6

Open canismarko opened 4 days ago

canismarko commented 4 days ago

Expected Behavior

It would be nice if we didn't need to always tell the Instrument which devices are fair game. The consumer code can do this if it wants, but to do this for all cases seems like boilerplate.

This should happen after the configuration file is parsed into the common definition list so that it doesn't depend on whether this done in TOML, YAML, or a custom configuration format.

The following example YAML file from @prjemian would then be usable. It would still require that either parse_config or parse_yaml_file is subclassed in order to understand the specific structure of the YAML file, but the device_class parameter would not need special treatment.

class: instrument.devices.temperature_signal.TemperaturePositioner
    kwargs:
      prefix: gp:userCalc8
      egu: C
      limits: [-20, 255]
    wait_for_connection: True

parse_config() or parse_yaml_file() would need to return the following python list:

[
    {
        "device_class": "instrument.devices.temperature_signal.TemperaturePositioner",
        "kwargs": {
            "prefix": "gp:userCalc8",
            "egu": "C",
            "limits": (-20, 255),
        },
    },
...
]

wait_for_connection might not be necessary since this can be handled by Instrument.connect(), but we can discuss separately.

Current Behavior

Currently, Instrument needs a dictionary of device class names and their corresponding Device class or factory:

instrument = Instrument({
    "motor": ophyd.EpicsMotor
})

which can then consume this TOML file:

[[ motor ]]
name = "m1"
prefix = "255idcVME:m1"

After parsing, the instrument looks up the section heading "motor" in the list of known device classes: https://github.com/spc-group/guarneri/blob/25dd1c00952e64fb612a4b459921e226519a2e43/src/guarneri/instrument.py#L158

Possible Solution

Let's say we have the following TOML:

[[ ophyd.EpicsMotor ]]
name = "m1"
prefix = "255idcVME:m1"

If the key for the device class (i.e. "ophyd.EpicsMotor") is not in device_classes, then the instrument could attempt to import it directly. I believe @prjemian has a way of doing this already so we can perhaps steal gain inspiration from that solution.

Context

Having the ability to specify device class names explicitly is useful for customizing the loading operation. But for many use cases this might be considered boiler plate, especially if the device is part of some other library (e.g. apstools) and isn't going to be subclasses or customized. Beamline staff could just add an entry to the configuration file directly using the import path and be done.

prjemian commented 2 days ago

The core of this capability comes as each device specification is parsed from the file (using just-added and unrelased apstools.misc.dynamic_import("import.by.absolute.dotted.class")).

    if class_name not in self.device_classes:
        self.device_classes[class_name] = dynamic_import(class_name)
prjemian commented 2 days ago

Perhaps the most general location for that code is before this line: https://github.com/spc-group/guarneri/blob/114d3c1a74329c0a46dfc24b4cc7de5854b844b8/src/guarneri/instrument.py#L153

so, rewriting the insertion:

    if defn["device_class"] not in self.device_classes:
        self.device_classes[defn["device_class"]] = dynamic_import(defn["device_class"])

An exception could be raised from dynamic_import() if the import fails for some reason. A later exception could be triggered if the imported Klass cannot be used in the context of obj = Klass(*args, **kwargs) to contruct an ophyd-style object.

prjemian commented 2 days ago

To avoid adding an apstools package requirement, guarneri could vendor the dynamic_import() function. It's very short.

canismarko commented 3 hours ago

To avoid adding an apstools package requirement, guarneri could vendor the dynamic_import() function. It's very short.

I agree. Dependencies get weird otherwise.