-
Notifications
You must be signed in to change notification settings - Fork 11
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
Parameterize Devices #18
Comments
One immediate thought I had (and I'm not sure I like it) was using environment variables. Something like: # <beamline>.py
import socket
from pydantic import BaseSettings
from .devices.simpledet import SimpleAreaDetector
from .devices.simstage import SimStage
class Settings(BaseSettings):
# $HOSTNAME
hostname: str = socket.gethostname().split(".")[0]
# $CHECK_CONNECTION
check_connection: bool = True
# $WARMUP_DETECTOR
warmup_detector: bool = True
stage = SimStage(name="sim_stage", prefix=f"{Settings.hostname}-MO-SIM-01:")
det = SimpleAreaDetector(name="adsim", prefix=f"{Settings.hostname}-AD-SIM-01:")
if Settings.check_connection:
stage.wait_for_connection()
det.wait_for_connection()
if Settings.warmup_detector:
det.hdf.warmup()
__all__ = ["stage", "det"]
# startup.py
from dodal.<beamline> import det This is fairly clunky and feels like it can be refined. It does present some level of flexibility but the path to changing settings is not obvious. We can't currently customize what happens depending on which devices are imported, I think that would require delving into the depths of importlib. Finally, there is lots of duplication between modules. We can move some of the settings to a global location but that adds complexity and more hidden magic. |
Another thought is to implement some form of lazy loading with the help of PEP 562 (https://peps.python.org/pep-0562/), that needs more thought. |
Is this a requirement? In DiamondLightSource/hyperion#200 (comment) we explicitly don't have this, instead requiring that you call |
Honestly, I would prefer |
I think mostly solved except I still don't see a neat way to do #6. |
this feels like an echo from the distant early days of dodal, might be solved with all that we have in #483 etc @DominicOram what do you think? |
Yeah, I think this is no longer relevant and can be closed |
excellent ❗ |
@DominicOram and @noemifrisina
Relates to #6, #3 and DiamondLightSource/hyperion#200
It seems, to avoid verbosity, we want module level devices so we can say
and immediately use
dcm
as oppose to having amake_dcm()
function that only instantiates the object when called.However, we may want the device to do slightly different things, depending on context, when we import it, for example:
wait_for_connection()
) so that it doesn't unexpectedly cause an error when first useddetector.hdf5.warmup()
) or not, or sometimes, as aboveIf we take parameterization as a possible solution for these use cases, we should think about how to achieve it. We want to maximize granularity and flexibility while minimizing boilerplate. The two are likely to work in mutual contravention so we would need to find a good compromise. Also, depending on exactly how much flexibility we feel we need, we should keep in mind the possibility that the
make_dcm()
function above may actually be the least worst option.The text was updated successfully, but these errors were encountered: