Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1877 +/- ##
=======================================
Coverage 99.05% 99.05%
=======================================
Files 312 312
Lines 11749 11761 +12
=======================================
+ Hits 11638 11650 +12
Misses 111 111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
oliwenmandiamond
left a comment
There was a problem hiding this comment.
"""A StandardReadable device representing a synchrotron facility.
This device provides access to key synchrotron parameters and operational
status information.
Attributes:
current (EpicsSignalR): Read-only signal for the synchrotron beam current.
energy (EpicsSignalR): Read-only signal for the beam energy.
probe (SoftSignalRW): Configurable signal for the probe type.
Defaults to "x-ray".
type (SoftSignalRW): Configurable signal for the synchrotron type.
Defaults to "Synchrotron X-ray Source".
synchrotron_mode (EpicsSignalR): Read-only signal for the current
synchrotron operating mode.
machine_user_countdown (EpicsSignalR): Read-only signal for the
machine user countdown timer.
top_up_start_countdown (EpicsSignalR): Read-only signal for the
top-up start countdown timer.
top_up_end_countdown (EpicsSignalR): Read-only signal for the
top-up end countdown timer.
"""
|
Here it is #1914 |
DominicOram
left a comment
There was a problem hiding this comment.
Thanks, I'm not a massive fan of docs like these. Like Read-only signal for the machine user countdown timer. gives me no additional context that the name machine_user_countdown doesn't already give me. If we want to write helpful docs then we should answer questions like "what is a machine user" maybe what units is the countdown in etc. As it stands it feels a bit like this is documenting things just for the sake of saying we have docs.
| signal_prefix (str, optional): Beamline part of PV. Defaults to Prefix.SIGNAL. | ||
| status_prefix (str, optional): Status part of PV. Defaults to Prefix.STATUS. | ||
| topup_prefix (str, optional): Top-up part of PV. Defaults to Prefix.TOP_UP. |
There was a problem hiding this comment.
Nit: I always thought having these as arguments was kind of silly. When would we ever change them?
There was a problem hiding this comment.
We probably will never change them, but if someone wants to reuse our code in a different synchrotron they would need it. Not that it is difficult to add though... That's the only reasoning I see behind it. But I get a feeling that we don't have such purpose of having this repo reused. Or do we?
I was more thinking of having more useful help() that gives list of Args and Attributes available. |
|
What is the recommended or agreed pydoc style in Diamond II developments? sphinx, google stype, numpy, or pydoc? do we need to support cross-reference in the doc string or not? |
Google style with few exceptions according to https://diamondlightsource.github.io/dodal/main/reference/style-guide.html |
Add docstring to synchrotron class.
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}