search

bluesky / ophyd

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

pip installation does not have all required dependencies #1170

Open marcomontevechi1 opened 1 year ago

marcomontevechi1 commented 1 year ago

When installing ophyd via pip i noticed simdetector and similar devices break:

>>> from ophyd.areadetector.detectors import SimDetector
>>> s = SimDetector("SIM:", name="SIM")
>>> s.cam.acquire.put(1)
Traceback (most recent call last):
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 327, in __get__
    return instance._signals[self.attr]
           ~~~~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'cam'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 327, in __get__
    return instance._signals[self.attr]
           ~~~~~~~~~~~~~~~~~^^^^^^^^^^^
KeyError: 'array_size_z'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 329, in __get__
    return instance._instantiate_component(self.attr)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1354, in _instantiate_component
    self._signals[attr] = cpt.create_component(self)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 266, in create_component
    cpt_inst = self.cls(pv_name, parent=instance, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 897, in __init__
    getattr(self, attr)
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 325, in __get__
    print(instance._signals)
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/ophydobj.py", line 580, in __repr__
    info = ", ".join("{}={!r}".format(key, value) for key, value in info)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/ophydobj.py", line 580, in <genexpr>
    info = ", ".join("{}={!r}".format(key, value) for key, value in info)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1644, in __repr__
    return repr(list(self))
                ^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1625, in __len__
    return len(self.__internal_list())
               ^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1607, in __internal_list
    return list(itertools.chain.from_iterable(out))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1602, in <genexpr>
    out = (
          ^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1142, in _get_components_of_kind
    yield component_name, getattr(self, component_name)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 329, in __get__
    return instance._instantiate_component(self.attr)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 1354, in _instantiate_component
    self._signals[attr] = cpt.create_component(self)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/device.py", line 266, in create_component
    cpt_inst = self.cls(pv_name, parent=instance, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/signal.py", line 1531, in __init__
    super().__init__(read_pv, string=string, name=name, **kwargs)
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/signal.py", line 972, in __init__
    self._read_pv = self.cl.get_pv(
                    ^^^^^^^^^^^^^^^
  File "/opt/micromamba/envs/teste-deletar/lib/python3.12/site-packages/ophyd/_dummy_shim.py", line 51, in get_pv
    raise NotImplementedError
NotImplementedError

This does not happen when installing via conda-forge. @flowln noticed that here there is a fallback to "dummy" in case neither caproto nor pyepics are installed. The error doesnt happen in conda-forge installations since they come with pyepics and caproto. Maybe adding these as dependencies in setup.py would fix this?

tacaswell commented 1 year ago

This was intentional as we do not want to have a hard requirement on a particular epics implementation (or any at all!) at the project metadata level (we also have an optional dependency on Matplotlib) because this is much harder to remove at a packaging (e.g. conda) level. On the other hand it is very easy for packaging ecosystems to add extra dependencies.

We also have the idea of supporting other control systems (e.g. https://github.com/bluesky/ophyd-tango) and in those use cases they should be able to depend on ophyd without picking up a transient dependency on another control system.

I am inclined to continue to not have a hard dependency, however I think adding extra groups for [caproto] and [pyepics] would be reasonable.

flowln commented 1 year ago

I agree with not adding a hard dependency, however I do think there should be a user-facing warning in the case the dummy layer is used, as it (as shown) can create issues that are not immediately obvious as to what the cause is (maybe adding an explanation to the NotImplementedError raised would be the best place for that).

tacaswell commented 1 year ago

We can not warn because we do not know we need to warn the user until we hit the error. We do not want to warn users who will never need epics.

(maybe adding an explanation to the NotImplementedError raised would be the best place for that).

I agree that is the best place for this!