-
Notifications
You must be signed in to change notification settings - Fork 62
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
Project structure for adding new devices #104
Comments
@TerryGeng some work was done a while ago to break the dependence on labscript devices. I believe it works, you can specify additional directories via the I'm not certain if this is better than either of the options you propose...maybe it's actually just complimentary? I wonder what the security implications would be if you just added the Python site packages folder to the Still, maybe the existing functionality could tie in with the package namespace feature? It wouldn't even have to be published under labscript_devices. You could publish a nist-labscript-devices package, add the top level package dir to the labconfig setting, and have the namespace packages contain the labscript device code (unless I've misunderstood how namespace packages work). Would be worth checking if they work with conda too... The upside to this is that you don't actually need to reach consensus here before you begin experimenting. You can try things out in a separate package and then keep us updated as to what works and what you think the best practices are! I'd love to see maintenance of device code become more distributed 🙂 |
I think this is a mixture of a philosophical and practical question. Should we bundle all of the well supported labscript devices into a single repo like we do now?
I think either is fine. However, I think it is a very poor idea to encourage people to setup their own repos like "nist-labscript-devices." Experience with python is that this leads to orphaned projects and difficulty finding packages of use. The problem @TerryGeng is trying to solve (in my view) is one of dependancies. If labscript-devices, as it is, were setup as a canonical python package it's install script would check for the dependancies of all of its supported devices (as it does now to some extent). I think this is (almost) always the right thing to do. However, this is in practice insane. If you don't have a physical device then you should not install the supporting packages/drivers. So we have the question: do we build error checking into the device framework that at runtime makes sure that a device's dependancies are installed and throws a helpful error if they are not, OR go the subpackage route, OR ....? P.S. The structure of @TerryGeng 's question also points out that the standard labscript practice of doing import * from pretty much every package is unwise. |
I want to spend more time thinking about this, but I have a few thoughts I wanted to inject now to the conversation. I'm definitely a big believer in getting user devices out there and easy to install for others. Labscript's utility derives from broad device support, so adding to that makes it easier for more users to get started. I firmly agree with Phil that moving to a more distributed model of device code maintenance would be very beneficial to this end. I also agree with Terry that namespace packages seem to be a decent way forward, though a smooth transition will probably be rather annoying. My not fully developed thought is that we should aim for a hybrid of the two methods Ian outlined:
In any case, I agree with Phil that it would be nice to have a test-bed to try out ideas that doesn't require modifying I have a few distinct thoughts responding to things already said that are informing this idea
Probably not what you mean, but I want to make sure we are being careful: I highly doubt we are allowed to host binary blobs from manufacturers directly in any repo. What this means is that installing many devices already requires tracking down 3rd party drivers (spincore for windows, all the cameras, VISA backends, etc), and the only python dependencies are their python wrapper packages which can be installed without fuss. Currently, having an unused device in As Ian astutely points out, this does mean you should never, ever call
While this experience is definitely true, I would argue it is a feature more than a bug (as far as orphaning goes). Given most devices are written by a single grad student who will move on in life, many orphaned labscript-device projects will lose their single maintainer and will be guaranteed to rot to unusable pretty quickly. It doesn't really matter where the code is hosted, it is orphaned all the same (see the AlazarTechBoard for instance). Having orphaned code live in another repo where it is obvious it is orphaned is preferable to me. If we want to move a device to mainline maintenance, we always can. And we can always maintain a list of such repos to make them easier to find (technically exists, but is admittedly not prominent nor complete). Another reason I prefer the separate repo approach is that many labscript-devices in use today are not written to the quality of
Maybe I'm missing a subtle distinction, but I believe this is largely already done. The camera devices are a good example. Runtime driver checks for dependencies only occur if the device is actually used within BLACS. Maybe the errors could be improved, but I think the standard 'could not import package' or 'binary not found' errors are pretty clear as to what needs to happen. There are other devices in |
Thanks for the good comments David. Regarding the last point, I would say that current devices do a really poor job of indicating a missing package, and do so in unique ways. For example in the IMAXdxCamear driver we have
where the nivision is the python wrapper for the NI camera code. When this is called without installing nivision, a messy python error comes back. In Pulseblaster we have this
For the AndorSolis we have
and then in AndorCam itself ctypes its way into the dll. So my point is there is not actually any dependency checking going on. We are just letting python fail and relying on its messages. I am suggesting an additional tool in labscriptutils that can be called when Blacs starts to initialize a device that checks for dependancies (that are enumerated by the author of the labscript device just like in setuptools, so no crazy code introspection). This could be checks for python packages, or external dll's. Thinking of external dll's probably we should allow the user to specify their location in the connection table rather than hard coding. |
Ok, I see. So there are multiple separate things here:
I'm not opposed to developing a standard for use inside the worker process for checking dependencies. It would definitely need to do the checks in the worker process though. I don't want to break the process sandbox model we have for dependencies. I'm not sure of how complex it would need to be to support all possible dependency types either. How far could we get simply by putting some
I don't think the connection table is the place for this. It doesn't need to know DLL locations, and definitely shouldn't be included in connection table comparisons when submitting shots. The local LabConfig file is available for PC specific settings. Devices are welcome to read their own section of the labconfig file if they want. |
I would like to thank everyone for the beneficial discussion here :). As @ispielma and @philipstarkey have clarified, we are discussing two problems: the structure of managing installations of different blacs devices and the dependencies check for some binary blobs required by some specific devices. I want to provide more thought about the former. I still believe the most proper way of managing device installation is to grant the user full freedom to decide which device to install, and which not to install. In this sense, the best way is to separate the entire This approach doesn't necessarily mean we cannot publish a pip package called Users can also decide not to install this standard distribution This will also allow other labscript users outside the labscript team to publish their own device packages, and could potentially attract some manufacturers to distribute their official Some other comments:
@philipstarkey In general, I'm somewhat against the idea of using hacks from
@ispielma I totally agree ^^. I'm intentionally removing it from all code I have edited.
@dihm I agree with you. While I think the idea of publishing a meta-package explained above can also fulfill this job without changing code in blacs and affecting the installation experience of users. |
@TerryGeng I'll admit I am definitely leaning more toward the namespace package method, and I do really like the vision of having each device be its own package that can be individually versioned and maintained. There are a few things we probably need to work out before really trying this. First, we need to ensure that all installation options are fully compatible with the namespace architecture. This includes the conda channel (docs are a bit slim on how to properly package for conda with namespace packages). It will also need to support running anywhere between all or none of the packages being installed in editable mode. It would be nice to find another python package that actually does these things successfully. Second, Third, while I agree at some level that playing import hijinks is not great, that was a compromise to ensure that users writing devices did not need to fully package their code to use it. Under an extreme version of this proposal that eliminated Finally, this would all need to work with the device_registry framework. I'm way less familiar with the details there and how hard or easy that will be. I also have a few lingering concerns personally. One is that it appears to be pretty easy for a namespace package to accidentally bork things (by accidental inclusions of |
Hi labscripts folks,
First, thank you all for providing such a wonderful experiment framework! Here at JQI, we are actively working on adding new devices to labscripts (strong push from @ispielma ^^). I'm currently finalizing a blacs device for DMDs using ALP 4.1 API (hopefully we can propose a general interface design for DMDs) and also a blacs device for GeniCam.
One technical difficulty I noticed is: every time I add one new device to labscript-devices repo, I will introduce a new bunch of dependencies (and sometimes, unfortunately, some binary files from the manufacturer). If some users don't really need the support of that device, these extra dependencies would just increase the difficulty of deployment.
To solve this problem, I have two solutions to propose:
device_a
anddeivce_b
and they can be imported byboth under the namespace
labscript_device
. In this way, the dependencies of each device will be maintained independently from other devices.git submodule
(one example is the way picosdk managing 3rd party libraries). We can pack all files, including binary files for that module, into an external repo and add that into labscript-devices as a submodule. When users decide to install this module, they only need to rungit submodule update --remote <path to the submodule>
. The downside of this method is it will mess up pip, and users need to manage the dependencies of that submodule by themselves.Let me know your thoughts about it!
The text was updated successfully, but these errors were encountered: