bluesky / ophyd-async

Hardware abstraction for bluesky written using asyncio
https://blueskyproject.io/ophyd-async
BSD 3-Clause "New" or "Revised" License
12 stars 26 forks source link

`SimMotor` is not importable without dev dependencies #502

Open callumforrester opened 3 months ago

callumforrester commented 3 months ago

I can't do: from ophyd_async.sim.demo import SimMotor in a project that depends on ophyd-async.

File ~/projects/bluesky/bluesky-stomp/venv/lib64/python3.10/site-packages/ophyd_async/sim/demo/_pattern_detector/_pattern_generator.py:4
      1 from pathlib import Path
      2 from typing import AsyncGenerator, AsyncIterator, Dict, Optional
----> 4 import h5py
      5 import numpy as np
      6 from bluesky.protocols import DataKey, StreamAsset

ModuleNotFoundError: No module named 'h5py'

h5py is a dev dependency, so it should move into primary dependencies?

coretl commented 3 months ago

I'm erring on the side of a ophyd_async[sim] extra that includes h5py

jwlodek commented 3 months ago

Should we re-organize imports to allow for importing SimMotor without needing h5py? As I understand h5py is only necessary for the pattern generator sim device. So SimMotor maybe should be importable w/o needing this library?

stan-dot commented 1 week ago

at the moment the main branch does have the ophyd_async[sim] extra

importing SimMotor without the h5py would only make sense if we restrict the methods --> if a method uses the h5py libarary we'd throw an error

import functools

def require_library(lib_name: str):
    """
    A decorator to ensure a specific library is available in the scope.
    If the library is not present, raises an ImportError.
    Caches the result for subsequent checks.
    """
    library_cache = {}

    def decorator(func):
        @functools.wraps(func)
        def wrapper(*args, **kwargs):
            # Check the cache first
            if lib_name not in library_cache:
                try:
                    # Attempt to import the library
                    __import__(lib_name)
                    library_cache[lib_name] = True
                except ImportError as e:
                    # Cache the failure and raise an error
                    library_cache[lib_name] = False
                    raise ImportError(f"Required library '{lib_name}' is not available.") from e

            # If library exists, run the decorated function
            if library_cache[lib_name]:
                return func(*args, **kwargs)

        return wrapper

    return decorator
coretl commented 1 week ago

I think we should organise sim into various subpackages that can each have their own deps, e.g.

sim\
  motor\
  detector\
  testing\
stan-dot commented 1 week ago

for now we just have 1 sim thing, is this a premature optimization?

callumforrester commented 1 week ago

I'm inclined to agree with @stan-dot, I don't think it is too costly to require the user to install h5py in order to make use of the sim package. Extra dependencies is a nice way to avoid requiring everything for every user but it's possible to go too far in the other direction too. H5py is one library and not too heavyweight, so I'd throw in my vote for requiring it if you want to use the sim package.

coretl commented 15 hours ago

Decision: make h5py a sim dependency and keep with demo