Skip to content
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

pip installation does not have all required dependencies #1170

Open
marcomontevechi1 opened this issue Oct 25, 2023 · 3 comments
Open

pip installation does not have all required dependencies #1170

marcomontevechi1 opened this issue Oct 25, 2023 · 3 comments

Comments

@marcomontevechi1
Copy link

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
Copy link
Contributor

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
Copy link

flowln commented Oct 25, 2023

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
Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants