Add representers for all channel access types #66
Conversation
| if isinstance(selected_values, dict): | ||
| selected_values = [selected_values] | ||
|
|
There was a problem hiding this comment.
I want to know people's opinion on this; while writing tests I found that, if get_signal_values is called using a dictionary containing just one device, the result of yield Msg("locate", ...) will be a dictionary, not a sequence. The type hinting for selected_values suggests it will always be a sequence. But it isn't... Personally I'd prefer if locate always returned a list of dicts, instead of sometimes returning a dict and sometimes returning a list of dicts.
Thoughts?
There was a problem hiding this comment.
@rosesyrett Since we are merging this for need then coming back to it, can I just put a type ignore on this to get mypy to behave? It's the only thing flagging now
There was a problem hiding this comment.
Yes please. The type hints are wrong; this is probably a bluesky bug.
|
|
||
| import numpy as np | ||
| import numpy.typing as npt | ||
| import yaml | ||
| from bluesky.plan_stubs import abs_set, wait | ||
| from bluesky.protocols import Location | ||
| from bluesky.utils import Msg | ||
| from epicscorelibs.ca.dbr import ca_array, ca_float, ca_int, ca_str |
There was a problem hiding this comment.
This is the right solution, but in the wrong place. We need to make sure epicscorelibs imports are only within the ophyd_async.epics packages.
How about a global (yuck) directory of representers that are added to on inport of packages?
There was a problem hiding this comment.
I think this needs a broader discussion on where stuff belongs. I don't like the fact that we have plans in ophyd_async.core, but definitely agree epics things shouldn't belong in ophyd_async.core. For the meantime I'd be keen to just merge this in anyway and come up with a better long term solution later...
There was a problem hiding this comment.
@coretl I suggest merging, making an issue and fixing later because of time constraints
abbiemery
force-pushed
the
add-representers
branch
from
bbfa88f
to
6941b40
Compare
abbiemery
force-pushed
the
add-representers
branch
from
3be15c1
to
e648a4c
Compare
we have save load functionality, but unfortunately it only works for PV access currently. So I've added some lines which let it work for channel access, by adding representers. I've also expanded on existing tests to ensure the serialisation/deserialisation works.