-
Notifications
You must be signed in to change notification settings - Fork 63
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
Make device specific dependencies 'extras' #47
Make device specific dependencies 'extras' #47
Conversation
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. |
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 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 |
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?
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? |
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 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. |
Ah, yes... not merging this doesn't preclude future dependencies being extras if they are objectionable.
+1 (which I neglected earlier) @philipstarkey do you concur on closing this? I'll move the useful bit of housekeeping this elicited to #49. |
I don't have strong opinions either way now so happy for this to be closed |
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
toextras_require
:If a user wants to control a National Instruments DAQ product and an IMAQdx camera they would install labscipt-devices using:
The keys for doing so could be based on device manufacturer, product line, SDK, or existing sub-module names, e.g.
NI_DAQmx
andIMAQdxCamera
instead of the above choices.This would allow bindings like
labscript_devices.AndorSolis.andor_sdk
andlabscript_devices.atsapi
to be independent packages.Are there any currently undeclared dependencies?