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

Idea: Add a human-readable "display name" to Ophyd Objects to supplement name #1161

Open
cjtitus opened this issue Oct 4, 2023 · 19 comments

Comments

@cjtitus
Copy link

cjtitus commented Oct 4, 2023

Background

Most of the time, the "name" attribute of my Ophyd objects is either auto-generated, i.e, "parentname_signalname", or at the top-level, something short and simple so that it's easy to remember and access in the run data later.

E.g, data['ref'] is easy to remember, data['SST Beamline Intensity Monitor'] is not

However, there are plenty of times when I would rather display "SST Beamline Intensity Monitor" to a user who isn't likely to care about the under-the-hood data key. This is becoming especially relevant when trying to make a GUI.

Suggestion

I think it would be very simple to add a display_name attribute to OphydObject. The implementation would probably be a property with a setter, that defaults to returning name if no display_name has been set.

I believe this would be very easy to implement, and completely non-disruptive. I would be happy to implement and submit a PR. Thoughts?

@ZLLentz
Copy link
Member

ZLLentz commented Oct 4, 2023

I think this would be useful in many cases

It's worth considering if there are any places internally where ophyd can and should leverage this information as well- does it belong in tracebacks and log messages, for example?

@cjtitus
Copy link
Author

cjtitus commented Oct 5, 2023

I'd say it could be useful in tracebacks/logs, but you'd have to be careful. The display name isn't necessarily going to tell you as much about the object structure as the actual name. So if you replaced name in the logs, you may end up with a situation where obj.subdevice.signal has a problem, but obj.subdevice.signal.display_name is something non-obvious like "Main Detector", whereas obj.subdevice.signal.name is going to be obj_subdevice_signal which tells you exactly what to look at.

Now granted, name isn't always particularly useful either...

I'd keep it simple for now and just add display_name, and let people choose how to use it

@tacaswell
Copy link
Contributor

"both" is probably the right answer for tracebacks and logs.

@dperl-dls
Copy link
Contributor

I have also often wanted this feature, and I agree, I would like to see both in logs and tracebacks

@coretl
Copy link
Collaborator

coretl commented Feb 28, 2024

Could we spell it description rather than display_name?

@danielballan
Copy link
Member

I wonder if description sounds too much like it would be the return value of the existing method describe().

@dmgav
Copy link
Contributor

dmgav commented Feb 28, 2024

Are component display names expected to be generated automatically based on display names of parents? Or display_name attribute should be set explicitly, otherwise it defaults to None/""?

@mrakitin
Copy link
Member

I'll add my 2 cents: we recently used the .DESC PV for the description used in CSS, example: https://github.com/NSLS-II-HEX/profile_collection/blob/069a23bd6fd069a0db5fcdc1916b5f30729b759a/startup/03-motors.py#L11-L13.

Docs: https://epics.anl.gov/EpicsDocumentation/AppDevManuals/RecordRef/Recordref-6.html. I don't think it's tied to the Motor Record only, seems to be a more general feature. @shroffk, could you please confirm?

@prjemian
Copy link
Contributor

In the apstools package, we define description as connected with the .DESC field.

@prjemian
Copy link
Contributor

In EPICS, .DESC is a field common to all records.

@shroffk
Copy link

shroffk commented Feb 28, 2024

As mentioned .DESC is a common field for all records.

With ca you need to retrieve this ca://my_pv_name.DESC ( additional connections...since you don't actively keep changing this you might not need to monitor this and just get away with a single get)
With pva the NT types support a description field which would be part of the meta data

    epics:nt/NTScalar:1.0
        double value
        time_t timeStamp
            ...
        structure display
            string description
            string units
            int precision
        ...

@cjtitus
Copy link
Author

cjtitus commented Feb 29, 2024

I'd like to comment that Ophyd objects are much closer to any notion of "intent", "purpose", or "description" when compared to the DESC field of an EPICS record. My intention is that the descriptions would be set on Bluesky startup, and that they would really describe an object, including its purpose, not just a generic string from EPICS.

As an example (which is very relevant to my beamline), if there are two endstations on a beamline, with a shutter in-between, one endstation may want to describe the shutter as "Downstream Photon Shutter", whereas the second endstation would call it "Upstream Photon Shutter". But perhaps (if they were organized scientists!) the ophyd name (i.e, not the description) would be something like "shutter4", based on some global beamline optical numbering scheme, which is important, but not terribly descriptive to a user, or as a label in a program.

Our Ophyd object names are typically unique, but the descriptions may not necessarily be. It should be up to the scientist writing the beamline code how to describe the objects in the most useful way for their circumstance.

@coretl
Copy link
Collaborator

coretl commented Feb 29, 2024

I wonder if description sounds too much like it would be the return value of the existing method describe().

How about label then?

As an example (which is very relevant to my beamline), if there are two endstations on a beamline, with a shutter in-between, one endstation may want to describe the shutter as "Downstream Photon Shutter", whereas the second endstation would call it "Upstream Photon Shutter". But perhaps (if they were organized scientists!) the ophyd name (i.e, not the description) would be something like "shutter4", based on some global beamline optical numbering scheme, which is important, but not terribly descriptive to a user, or as a label in a program.

I agree that the EPICS .DESC field is generally not useful at the moment, and there is not a clear map for that to compound devices like detectors or shutters. However it could be useful at the signal level. For your shutter device for instance then let's assume we have setpoint and readback Signals, then we could have:

  • shutter4.label = specified when constructing Shutter ophyd class = "Downstream Photon Shutter"
  • shutter4.setpoint.label = value of <setpoint_pv>.DESC = "Shutter desired state"
  • shutter4.readback.label = value of <readback_pv>.DESC = "Shutter actual state"

@jwlodek
Copy link
Member

jwlodek commented Feb 29, 2024

I think label is a good name that shouldn't collide with anything.

@flowln
Copy link

flowln commented Feb 29, 2024

Wouldn't label be too similar to OphydObject's _ophyd_labels_ attribute though? It would make things confusing IMO.

I like display_name, it seems simple and intuitive.

@prjemian
Copy link
Contributor

FWIW, NeXus has a long_name attribute that matches this use. It is defined in many NeXus classes, such as NXdata.

@prjemian
Copy link
Contributor

And, yes, NeXus also had long discussions about how to name things.

@prjemian
Copy link
Contributor

I believe long_name here might be misleading (by association with the ophyd object .name). Perhaps long_description would fit the original intent, without association to the the EPICS .DESC field?

@cjtitus
Copy link
Author

cjtitus commented Feb 29, 2024

I actually like long_name, and I think the association with ophyd object .name is good. My whole motivation for this is that we have stupid ophyd objects where .name = "Exit Slit of Mono Vertical Gap". And I want to reconfigure things such that .name = "eslit" and then .long_name = "Exit Slit of Mono Vertical Gap" makes a whole lot of sense.

I also think it would be good if we inched towards making it easier to export NeXus classes from Tiled data.

In contrast, I think long_description would be confusing, because there is currently no "short description", other than describe, which is of course totally different.

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