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

Allow device classes to be specified as direct imports #6

Open
canismarko opened this issue Nov 18, 2024 · 4 comments
Open

Allow device classes to be specified as direct imports #6

canismarko opened this issue Nov 18, 2024 · 4 comments
Assignees

Comments

@canismarko
Copy link
Contributor

canismarko commented Nov 18, 2024

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:

Klass = self.device_classes[defn["device_class"]]

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
Copy link
Collaborator

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
Copy link
Collaborator

prjemian commented Nov 20, 2024

Perhaps the most general location for that code is before this line:

Klass = self.device_classes[defn["device_class"]]

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
Copy link
Collaborator

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

@canismarko
Copy link
Contributor Author

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

I agree. Dependencies get weird otherwise.

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

3 participants