-
Notifications
You must be signed in to change notification settings - Fork 0
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
Convert Lists to sets and update ruff command #78
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.
Not all the changes are required here, but they don't hurt either way as we're just treating like collections in any case
src/i22_bluesky/plans/stopflow.py
Outdated
@@ -206,7 +204,7 @@ def stopflow( | |||
|
|||
stream_name = "main" | |||
flyer = HardwareTriggeredFlyable(StaticSeqTableTriggerLogic(panda.seq[1])) | |||
devices = [flyer] + detectors + [panda] + baseline | |||
devices = {flyer} | detectors | {panda} | baseline |
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.
devices = {flyer} | detectors | {panda} | baseline | |
devices = {flyer, panda} | detectors | baseline |
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.
Done
src/i22_bluesky/plans/stopflow.py
Outdated
"detectors": {repr(device) for device in detectors}, | ||
"baseline": {repr(device) for device in baseline}, |
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.
I think this should become {device.name for ...
, as the repr for the devices is unuseful and with the device references the name is closer to referencing what we're actually using.
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.
changed to device.name + ":" + repr(device) for ...
as discussed.
src/i22_bluesky/stubs/load_save.py
Outdated
@@ -1,5 +1,6 @@ | |||
import os | |||
from pathlib import Path | |||
from typing import Set |
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.
Use set
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.
Ok
tests/plans/test_stopflow.py
Outdated
@@ -1,4 +1,5 @@ | |||
import asyncio | |||
from typing import Set |
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.
set
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.
Ok
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.
one potential nit, but I'm happy to make the f-string change it's own thing later on
"panda": panda.name + ":" + repr(panda), | ||
"detectors": {device.name + ":" + repr(device) for device in detectors}, | ||
"baseline": {device.name + ":" + repr(device) for device in baseline}, |
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.
"panda": panda.name + ":" + repr(panda), | |
"detectors": {device.name + ":" + repr(device) for device in detectors}, | |
"baseline": {device.name + ":" + repr(device) for device in baseline}, | |
"panda": f"{panda.name}: {repr(panda)}", | |
"detectors": f"{detector.name}: {repr(detector)}" for detector in detectors}, | |
"baseline": f"{device.name}: {repr(device)}" for device in baseline}, |
Change function params which actually should be Sets (e.g. selction of detector) to Sets rather than Lists as the currently are. THis makes the JSONSchema for plans produced by BlueAPI more accurate. Also update ruff command from ruff to ruff check