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

Should Devices have an ophyd sync compatible repr? #709

Open
coretl opened this issue Dec 17, 2024 · 7 comments
Open

Should Devices have an ophyd sync compatible repr? #709

coretl opened this issue Dec 17, 2024 · 7 comments
Milestone

Comments

@coretl
Copy link
Collaborator

coretl commented Dec 17, 2024

At the moment bps.scan does this for the args:
https://github.com/bluesky/bluesky/blob/f655a730887e813c1905f89281cb564dd88d4e88/src/bluesky/plans.py#L1251

@burkeds reports:

It looks like ophyd devices are returning string representations of themselves differently than ophyd devices.
For example, in scan, the list of motors is populated by list(map(repr, [motor1])) which returns something that looks like:
["SynAxis(prefix='', name='motor1', read_attrs=['readback', 'setpoint'], configuration_attrs=['velocity', 'acceleration'])"]
An ophyd-async device returns the simple string representation of the object:
['<ophyd_async.sim.demo._sim_motor.SimMotor object at 0x7f2be5339510>']
Should ophyd-async devices have a similar return? This difference is breaking our analysis pipelines when moving between ophyd and ophyd-async devices

@tacaswell @danielballan thoughts?

@tacaswell
Copy link
Contributor

I am in favor of having nice reprs, but am a bit worried that analysis pipelines are depending on the exact details of the repr. Everything you could want to know about the scan that are in that list should be available in a more machine-friendly form somewhere else in the documents.

@coretl
Copy link
Collaborator Author

coretl commented Jan 6, 2025

In which case I propose we add a repr with just the name for ophyd async: SimMotor(name='motor1')

@burkeds what exactly are the analysis pipelines reading from the repr? Could they get the same information elsewhere in the doc as @tacaswell suggests?

@tacaswell
Copy link
Contributor

To take a slightly harder stance, in general I do not think that parsibility of repr/str output is part of the "stable API" and can be broken at any time as the target consumer of these is mostly humans (our devices are for the most part too complex to be good candidates for round-trippable reprs, but even if they were the target is Python not ad-hoc parsing).

@burkeds
Copy link
Collaborator

burkeds commented Jan 7, 2025

We are just reading the names of the devices in detectors and motors.

Here in the bluesky documentation (https://blueskyproject.io/bluesky/main/metadata.html) you defined a set of "standard" fields present in all bluesky.plans. Important for our purposes, this includes the detectors and motors fields. This means that you have effectively created a standard method for defining dependent and independent axes in the start document outside of device definitions.

This naturally means that any bluesky analysis pipelines can be expected to use this standard set of metadata that has been defined. Especially if those pipelines are intended for multi-beamline or multi-facility use. Since these bluesky.plans rely on repr, it stands to reason that to comply with this implied standard, devices must also make careful use of repr.

I do not think that this is an ideal approach, however this seems like a natural consequence of choices made when defining bluesky.plans.

@burkeds
Copy link
Collaborator

burkeds commented Jan 7, 2025

I'll also confirm that the people here working on analysis of Bluesky documents have interpreted the presence of this "standard" set of metadata in the plans to mean it is the "official" Bluesky metadata scheme that should be complied with and that schemes which do not comply with it are not "real" Bluesky schema.

@coretl
Copy link
Collaborator Author

coretl commented Jan 7, 2025

Here in the bluesky documentation (https://blueskyproject.io/bluesky/main/metadata.html) you defined a set of "standard" fields present in all bluesky.plans. Important for our purposes, this includes the detectors and motors fields. This means that you have effectively created a standard method for defining dependent and independent axes in the start document outside of device definitions.

It looks like the motors key is the names of the motors, which doesn't rely on repr, so is safe:
https://github.com/bluesky/bluesky/blob/f655a730887e813c1905f89281cb564dd88d4e88/src/bluesky/plans.py#L1265

However the detectors key has the repr of those objects:
https://github.com/bluesky/bluesky/blob/f655a730887e813c1905f89281cb564dd88d4e88/src/bluesky/plans.py#L1256

@tacaswell is there an alternative way to get the names of the detectors?

@coretl
Copy link
Collaborator Author

coretl commented Jan 13, 2025

@coretl coretl added this to the 1.0 milestone Jan 13, 2025
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