ENH: Add device descriptions #124
Conversation
…ll string handling
|
from the jira ticket:
It seems like this is implemented per device, where a device may be composed of many motors. I bring this up because TMO probably wants to annotate their motors on their IP1 endstations: So if this is the way to go then we need to adjust how TMO is represented in happi |
| from qtpy.QtWidgets import QGridLayout, QMenu, QPushButton, QWidget | ||
| from qtpy.QtWidgets import (QGridLayout, QHBoxLayout, QLineEdit, QMenu, | ||
| QPushButton, QWidget, QWidgetAction) | ||
| from ruamel.yaml import YAML |
There was a problem hiding this comment.
Should we consolidate all yaml parsing to use ruamel in a future PR? It's kind of strange to use two different libraries for the same thing.
There was a problem hiding this comment.
ruamel is only really necessary for writing the file, if we want to preserve comments / spacing etc. For reading pyyaml is sufficient. It's probably good to be consistent but I thought it wasn't too important
| config['DEVICE_HINTS'].pop(device_name, None) | ||
| else: | ||
| config['DEVICE_HINTS'][device_name] = device_desc | ||
| ryaml.dump(config, Path(fpath)) |
There was a problem hiding this comment.
I think this is the first time in lucid where the user config file is written back to by the program.
Could this create issues where someone is editing the user config file and someone else is setting descriptions? Maybe.
I think we should consider using a second file for this sort of extra data.
There was a problem hiding this comment.
Putting it all in the same file is probably the easy way out. I suppose the case where multiple lucid's are open at the same time will hit upon this rather quickly.
This reminds me of the issue we had in happi where a crash interrupts writing and wipes the file. I should do the write-new-file -> copy over sequence we implemented there.
There was a problem hiding this comment.
To me, this feels qualitatively different to happi which has a cli tool for editing the file, whereas in this project the user is expected to edit the file themselves.
There was a problem hiding this comment.
I commented on the typhos PR before making it back to this one... Separating the files makes sense.
Backing up a step, though...
You mention happi, @ZLLentz, did you guys discuss about putting these notes directly into the database? It seems sensible to have shared notes in a database to me. Or is it like different hutches should have different notes about a shared beamline component or something?
There was a problem hiding this comment.
I think that happi could become a source for notes, but at a lower priority (shadowed by local or env var notes). This is precisely for the reason you mentioned; different hutches might be looking at the same beamline component and have different notes, and I wouldn't want them to clobber each other.
The other wrinkle is that happi makes sense for top-level device information, but storing component level notes in happi might get a bit messy (though not completely unreasonable).
There was a problem hiding this comment.
I feel like we've talked about storing extra data in happi as a go-to per-device data source a bunch of times in many contexts, which makes me think that it's worth doing. It might even be worth storing subdevice notes along with the device notes, I'm not sure? It's not immediately clear to me and warrants some discussion and mock-ups.
There was a problem hiding this comment.
That's a lot of sources of notes and documentation... Off the top of my head, correct me if I'm wrong:
- LUCID config file 1 - env var notes
- LUCID config file 2 - local notes
- typhos config file 1 - env var notes
- typhos config file 2 - local notes
- happi database
- confluence - PCDS
- confluence - mechanical team / hutch team / etc
- FRS / ESD / ...
- Design review slides...
There was a problem hiding this comment.
To some extent the Typhos PR could supercede this one, or at least LUCID/Typhos could use the same files. (so maybe that simplifies things slightly)
Adding yet another source of documentation that can fall out of date leaves a slightly bitter after taste, but I could argue it fills a slightly different niche
| @@ -7,4 +7,5 @@ pyqt5>=5 | |||
| pyqtads | |||
| pyyaml | |||
| qtpy | |||
| ruamel.yaml | |||
There was a problem hiding this comment.
unrelated: why are package names allowed to contain periods?
There was a problem hiding this comment.
I don't like it either, but it's a pretty widely used package so it seems
I see, that's a bit more involved then. If we wanted to add a description field to device components, that should probably target Perhaps in the lucid-devices case, this isn't a common occurrence. I can follow up with MingFu to clarify |
|
The other way we can handle this is to rework the TMO happi entries to have a separate entry for each of these endstation motors. I think some people might argue that this is better because it lets you open up the motors separately if needed. |
|
superceded by pcdshub/typhos#557 |
Description
Adds a device description field that saves information to the toolbar yaml file.
I'll admit I kinda hacked things together here but I'd procrastinated on this long enough
To-do:
Context
https://jira.slac.stanford.edu/browse/ECS-1039
Screenshots