-
Notifications
You must be signed in to change notification settings - Fork 10
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
make device factory decorator #597
make device factory decorator #597
Conversation
94124b4
to
27c1b79
Compare
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.
No idea if the rest of the code is right until some tests are written...
To clarify, the code in the original ticket is an idea, but has never been run or tests. It is expected that it is wrong in some way, and writing tests to exercise it will reveal how it is wrong... |
docs/developer/explanations/decisions/0003-added-make-devices-decorator.rst
Outdated
Show resolved
Hide resolved
f2ad915
to
fb5e142
Compare
26368ca
to
6cb1db6
Compare
do we want to add 'skip device' to the factory too? we could have just one decorator |
This is a question for @callumforrester and @DominicOram. I don't know what a |
|
it's an architectural question. We can have all the logic that belongs to 'making device class instances' inside a bunch of nested decorators that need to be in right order. OR we can have 1 factory class that accepts a dictionary of features (lazy, mock, skip, directory provicder, post-create script) in one place. |
I have a weak preference for a single decorator, but defer to @callumforrester and @DominicOram for a decision |
Single decorator works for me. |
ok I'll prepare a proof of concept for a single decorator |
6cb1db6
to
49e3bd2
Compare
beamline_utils.py 180 but that is inside ophyd, not sure how it works for ophyd-async |
0ecf386
to
656f918
Compare
b185f4c
to
c28c7c4
Compare
addrressed
Replaced by #841: thanks Stan for all your work with this PR: I recognise how frustrating it is to be so close for so long, and we learned a lot about the requirements of the change in the process. |
Fixes #483
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}