labscript-suite / labscript-devices

A modular and extensible plugin architecture to control experiment hardware using the 𝘭𝘒𝘣𝘴𝘀𝘳π˜ͺ𝘱𝘡 𝘴𝘢π˜ͺ𝘡𝘦.
http://labscriptsuite.org
Other
5 stars 58 forks source link

Make device specific dependencies 'extras' #47

Closed rpanderson closed 4 years ago

rpanderson commented 4 years ago

Rather than include dependencies specific to particular labscript device driver(s) in install_requires, use setuptools 'extras' instead.

This will allow users to choose which additional dependencies to include when installing labscript-devices. Specifically, this PR sees the following moved from install_requires to extras_require:

[options.extras_require]
spincore = spinapi
nidaqmx = PyDAQmx
nivision = PyNIVision

If a user wants to control a National Instruments DAQ product and an IMAQdx camera they would install labscipt-devices using:

$ pip install labscript-devices[nidaqmx,nivision]

The keys for doing so could be based on device manufacturer, product line, SDK, or existing sub-module names, e.g. NI_DAQmx and IMAQdxCamera instead of the above choices.

This would allow bindings like labscript_devices.AndorSolis.andor_sdk and labscript_devices.atsapi to be independent packages.

Are there any currently undeclared dependencies?

philipstarkey commented 4 years ago

Looks like there are a bunch of dependencies missing. I haven't done a complete search, but things like zaber API and pyvisa are missing. I'm uncertain whether these should be optional or not - probably it makes sense to keep the optional ones for those that require extra device drivers to be installed? Otherwise we should be splitting off pyserial and pillow into optional install as well and then it perhaps starts getting messy. I guess an argument could be made that this PR is unnecessary if the 3 mentioned packages install fine even if you don't have the drivers installed. I'm conflicted now!

Another dependency is the OpalKelly library but that isn't even on PyPI (last I checked). It gets installed to your default Python when you install the driver and you have to copy it into your virtual environment.

chrisjbillington commented 4 years ago

I guess an argument could be made that this PR is unnecessary if the 3 mentioned packages install fine even if you don't have the drivers installed.

I'll make that argument :p

I'd like to take the batteries-included approach of including everything we use that is allowed by licensing and can install regardless of whether the user has OS device drivers/etc. Especially if the code exists on PyPI already so that we can just pull it in. This way the user gets a more out-of-the-box experience.

We don't draw the line at optionally installing labscript drivers that you aren't using, why draw it at pulling in the 3rd party library used by that labscript code?

If it weren't impractical/license-incompatible to ship the vendor's C libraries and OS drivers (the latter probably not possible with a package manager like pip or conda), I would want to ship them too. for a truly out-of-the-box experience. For the specific case of spincore we actually do ship the compiled SpinAPI on Linux with the spinapi python package, this is pragmatic because Spincore doesn't ship a compiled Linux library so we're saving users from having to compile it.

The only downside I see of pulling in additional packages is the risks that come with additioinal dependencies at install time - slightly higher risk of creating an incompatible set of dependencies, and slightly higher risk that one of the packages will be broken at install time and prevent users from installing the labscript suite even though they're not using that code.

We can leave out libraries that really do have incompatible dependencies, e.g. the ones that only work on a specific Python version. Or more to the point, we can include them with a ; py_version == 'x.y' qualifier so that pip doesn't attempt to install them on incompatible platforms - and convention is for the labscript_devices code for these devices to raise an error about the Python version on import.

rpanderson commented 4 years ago

It depends on which side of labscript-devices as a plugin framework vs labscript-devices as a burger-with-the-lot you fall on. Or is this false dichotomy?

The only downside I see of pulling in additional packages is the risks that come with additioinal dependencies at install time - slightly higher risk of creating an incompatible set of dependencies, and slightly higher risk that one of the packages will be broken at install time and prevent users from installing the labscript suite even though they're not using that code.

This alone is sufficient grounds to consider this PR, I think. And this speaks to the "labscript-devices as a plugin framework" design, where you shouldn't need a bunch of plugins (and packages) you're not using, especially if they jeopardise stability. What happens if someone wants to merge a device with dependencies that aren't agreeable?

chrisjbillington commented 4 years ago

I think it's something of a false dichotomy. Implementation as plugins allows people to hack their own things on and contribute them in an obvious way and forces us to keep concerns separated in code. But that doesn't mean they can't ship together by default.

Since we have continuous integration now, we will notice if one of our dependencies updates its dependencies in a way that creates a conflict, and then we can just make the source of conflict an optional dependency. We can use our judgement and make optional unreliable projects, or things that we can't depend on for licensing reasons, or particularly fat downloads optional. But I'd like to include as much as possible otherwise. I don't think this will happen very often. Also, wheels cannot fail at install time other than via dependency conflict.

An out-of-the-box experience is very nice, and I'm picturing users getting an ImportError (which we will need to wrap to tell them what to type into a terminal to fix it), and going to a terminal and get the syntax right. It's one of those examples of the computer telling you to do something that involves no choices on your part - the computer knows exactly what you want, and it has the power to do it, so why didn't it do it? The syntax of installing optional dependencies is an extra thing the user will have to figure out that we could avoid the majority of the time.

I think this drawback is bigger than the drawback of packages unexpectedly creating version conflicts, which I think will be pretty rare.

So I don't think it adds much to exclude by default. I think we should include by default, unless we have reason not to.

rpanderson commented 4 years ago

Ah, yes... not merging this doesn't preclude future dependencies being extras if they are objectionable.

An out-of-the-box experience is very nice,

+1 (which I neglected earlier)

@philipstarkey do you concur on closing this? I'll move the useful bit of housekeeping this elicited to #49.

philipstarkey commented 4 years ago

I don't have strong opinions either way now so happy for this to be closed