-
Notifications
You must be signed in to change notification settings - Fork 6
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
Converted ophyd enum types to uppercase. #320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks good but I think we can change the names of the enum values.
uA_V = "uA/V" | ||
mA_V = "mA/V" | ||
PICOAMP_PER_VOLT = "pA/V" | ||
NANOAMP_PER_VOLT = "nA/V" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't we using numerals for this? E.g. "1_HUNDRED"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted message when opening Ion Chamber windows in firefly:
[ERROR ] - Can not change value to <ScanInterval.SCAN_0_2: '.2 seconds'>. Not an option in PyDMComboBox
Possibly related to the changes here, but also maybe not. Overall though, everything appears to work in spite of error messages.
Yes, actually it is. The options for the labjack scan parameter is actually ".2 second" not ".2 seconds". I'll fix it. |
I think that error happens when the PyDM widget tries to set the value from the PV before the enum combobox has updated the list of available options. I don't think it's related to this PR, so I will merge this one, and put this bug into a separate issue for later: #330 Thanks @sterbinsky! |
The next version of ophyd-async will enforce capitalization for attributes on enum types. Currently, these are recommended by convention, and so doing this now shouldn't cause any backwards compatibility issues.
Things to do before merging:
write docsupdate iconfig_testing.toml