bluesky / ophyd

hardware abstraction in Python with an emphasis on EPICS
https://blueskyproject.io/ophyd
BSD 3-Clause "New" or "Revised" License
49 stars 78 forks source link

v2: non-descriptive error for epics_signal factories if aioca or p4p not installed. #1113

Closed rosesyrett closed 1 year ago

rosesyrett commented 1 year ago

Summary of Problem

If aioca is not installed, making an epics_signal_r (or rw, or x .. etc) will fail with a non-descriptive error message:

AttributeError: 'function' object has no attribute 'value'

This is unexpected behaviour and the error is obscure.

Description, with examples

If aioca or p4p are not installed, ophyd/v2/epics.py has:

try:
    from ._aioca import CaSignalBackend
except ImportError as ca_error:

    def CaSignalBackend(*args, ca_error=ca_error, **kwargs):  # type: ignore
        raise NotImplementedError("CA support not available") from ca_error

and equivalent for p4p. ._aioca and ._p4p contain import p4p, and import aioca, which will fail with the import error. So if my venv doesn't have aioca for example, CaSignalBackend will be a function, not a class.

The issue with this, is later on if you want to make backends (as epics_signal_r does, or any epics signal factory)...

def _transport_pv(pv: str) -> Tuple[EpicsTransport, str]:
    split = pv.split("://", 1)
    if len(split) > 1:
        # We got something like pva://mydevice, so use specified comms mode
        transport_str, pv = split
        transport = EpicsTransport[transport_str]
    else:
        # No comms mode specified, use the default
        transport = _default_epics_transport
    return transport, pv

def _make_backend(
    datatype: Optional[Type[T]], read_pv: str, write_pv: str
) -> SignalBackend[T]:
    r_transport, r_pv = _transport_pv(read_pv)
    w_transport, w_pv = _transport_pv(write_pv)
    transport = get_unique({read_pv: r_transport, write_pv: w_transport}, "transports")
    return transport.value(datatype, r_pv, w_pv)

_transport_pv should return a tuple of an enum and string. However,if I don't have aioca installed, CaSignalBackend is replaced with a function, which apparently isn't stored properly in the enum. This means that the call to transport.value fails, because transport is then just a function, instead of an Enum. In ipython...

In [19]: EpicsTransport.pva
Out[19]: <EpicsTransport.pva: <class 'ophyd.v2._p4p.PvaSignalBackend'>>

In [20]: EpicsTransport.ca
Out[20]: <function ophyd.v2.epics.CaSignalBackend(*args, ca_error=ModuleNotFoundError("No module named 'aioca'"), **kwargs)>

In [21]: _transport_pv("pva://abc")
Out[21]: (<EpicsTransport.pva: <class 'ophyd.v2._p4p.PvaSignalBackend'>>, 'abc')

In [22]: _transport_pv("abc")
Out[22]: 
(<function ophyd.v2.epics.CaSignalBackend(*args, ca_error=ModuleNotFoundError("No module named 'aioca'"), **kwargs)>,
 'abc')

How to fix this

In this PR I mentioned to avoid typing errors, if aioca/p4p are not installed, instead of replacing CaSignalBackend and PvaSignalBackend with functions we should replace them with classes. I think this is now a better idea as it would solve this issue. But am open to discussion on anything else we should do.