Third-Party Devices #22

Open philipstarkey opened 7 years ago

Original report (archived issue) by David Meyer (Bitbucket: dihm, GitHub: dihm).

In the course of setting up my lab I've written a few new device classes for working with labscript. Right now I have simple classes for use with Tektronix scopes and single-tone frequency synthesizers (think big HP boxes and the like) based on a more generic VISA class in the VISAinstr branch of my fork that I would like to contribute upstream to help widen device support for labscript. I've talked to Chris about this in person but figured I should open up the audience before sending pull requests directly to the repository. The obvious issue is that you need to be careful about having code in the repository that you can't test directly.

Could I suggest at least a third-party subfolder in the labscript_devices repo or maybe even a separate repository for holding such contributed devices? The idea is to make it easy (and obvious how) to contribute back while also making it clear the devices are not directly supported.

Original comment by David Meyer (Bitbucket: dihm, GitHub: dihm).

Edited issue description Edited description to add the link.

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).

I'd widely support more devices being made available! We also have implemented a synthesizer (HP_8672A) and a Tektronix scope(Tektronix_TDS210) along with a bit of software for plotting and so on. As I've not personally written that code nor used it much so I'm a bit hesitant of adding it. But non the less it seems that everyone uses about the same devices. A devices repo would definitely help everyone starting out and could also reduce the workload of implementing devices someone already has.

I think a separate repo would be the best way to go.

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

Yes we should solve this. Tangentially, I've been having discussions with @shaunj regarding how we might manage user contributed plugins for BLACS (and hopefully other programs). It's a similar problem (though to be clear, I am not proposing devices and plugins be related in anyway).

A proposed structure we came up with for this sort of user contributed code would be something like (replace __ with device code or plugins or analysis scripts etc.):

A set of __ included with the main master repositories of the labscript suite (as we currently do)
A set of repositories managed by the labscript_suite team for __, grouped under a new project (bitbucket repository hierarchy currently has team -> project -> repository. Currently all of our repositories are in the one project with the same name as the team). These repositories would have access delegated to specific people outside the core team who maintain them (e.g. the maintainer for the device code). I think at this level, I'd suggest maintainers were limited to people who have shown a willingness to contribute to the main project, and are familiar with the coding conventions and quality expected (aka the people discussing here). This bypasses the need for us to have to vet every pull request like we're doing the the moment, which is particularly important if we can't test! Emphasis would need to be placed on never breaking backwards compatibility with previous version of the exposed user API, so as not to make life difficult for people using code from these repositories.
__ code stored in other repositories (aka their own and unrelated to the main repositories)

This suggests having a simple GUI to manage the pulling of code from any/all of the above repositories. I guess this is basically like a simple package manager, where you can add different sources which may have varying qualities, and then install the __ that you want. Such a system would also allow us to at least warn users about name conflicts.

I'd also suggest it is an open question as to whether the concept outlined in point 2 above would have one repository per __ or one repository containing multiple __. The former makes it easier to assign which people maintain which bits of code.

Anyway, that's approximately an idea I had for this sort of thing. What do people think to it?

Original comment by David Meyer (Bitbucket: dihm, GitHub: dihm).

I think this is a good way forward personally. As far as point 2, I would think one repo per set of distinguishable ___. I say this because I've intentionally written my VISA based devices as children of parent classes to ease long term maintenance. I'd prefer to have all the inheritors in the same repo, even though they cover different devices. I suppose this would be less important if the simple package manager existed and had good dependency resolution.

That's my two cents.

Original comment by David Meyer (Bitbucket: dihm, GitHub: dihm).

As a note to myself for when we finally get around to doing this, the ClassRegister functionality is going to need some moderate reworking in order to find devices not in the main repository (the module search is basically hard-coded in the __getitem__() method now). It took me longer than it should have to figure this out and I don't want to forget.

Original comment by David Meyer (Bitbucket: dihm, GitHub: dihm).

@chrisbillington

Does pull request #51 at least partially resolve this?

Right now I've got a hack on the ClassRegister to find our own device repo outside the labscript_devices folder. Is the idea now to subrepo our devices into the labscript_devices folder with a register_classes.py in our device repo? Do I need to move over all of our old import style code to the new style for this to work well?

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

It does address it somewhat.

Subrepos would work.

Another option for including in-development devices would be to have labscript_devices scan additional directories for 'register_classes.py' files. For example, we could have a path settable in labconfig for where additional devices are located. Or, we could just have all of userlib be scanned (it's not like there's much of a performance hit for this).

I'm eager to have 3rd party devices 'upstreamed', and with the possibility of subrepos, it makes it pretty transparent - such that the developers of the code are in charge of their own code whilst it still being included in mainline. But I think if devices are to be included in mainline labscript that are not maintained by the core developers, I'd prefer to still have one repo, and just allow the maintainers of that code to be in charge of it - reviewing each others' pull requests or whatever, with the core developers just rubber-stamping or raising style issues.

To get existing device code into mainline, or to have it work either in a subrepo or a separate folder, yes, you'd need to convert it to the 'new' way of doing things with 'register_devices.py' files etc. It makes sense to put a bunch of related devices all in one folder of labscript_devices, the folder structure is free, we should do whatever is sensible. For example the DAQmx code I'm working on is all within one folder, though it is for all DAQmx models and has separate classes for all of them (see here for what that's looking like). Devices that are not converted to the new way of doing things will only continue to work if they are at the top level of labscript_devices, though of course you're free to re-add your hack to make old-style devices keep working elsewhere. But we will presumably eventually remove this 'old-style' way of looking up devices.

So how about this:

We make labscript_devices scan userlib, allowing in-development devices to be developed there, or entirely separate repos of devices to be maintained separately from the labscript_devices repository, if the code is not appropriate to share for whatever reason (though I really want to lower the bar for this - please share, even if the code is not the best quality, it can improve after it is out there. I'm more thinking there's no reason to share code for in-house devices, that sort of thing).
We agree that it is up to the users of a particular device maintain it, and defer to them to be the judges of how much testing is enough, before merging any given change. Core developers will still do the merging though, unless clicking the merge button in response to "this is ready for merging" comment actually consumes too much of our time. Actually I have been doing all 3rd party merges locally to get around a deficiency in bitbucket, so I would have to share the tools I use to do that before others could do the same, or at least make sure the process is understood, as it's not quite as simple as clicking the merge button.

In the DAQmx support I'm working on at the moment, I've broken backward compatibility with previous DAQmx device classes, so I added a __version__ attribute to be able to detect this and raise meaningful errors. In this way we can track the versions of individual labscript devices regardless of them being in the same repository, and we can mark them with a version number less than one if they are not yet considered stable.

At some point I'll be making a push for automated sphinx documentation from docstrings, at that point we can add extra information to the documentation like: "who is maintaining this device code?", "which labs use this device that are happy to test?" etc, these sorts of things could aid development as you will know who can test, who you might be breaking compatibility for etc.

Original comment by David Meyer (Bitbucket: dihm, GitHub: dihm).

All of this sounds good to me, having not thought about it super carefully.

For the near term I really like the idea of just designating a separate "local devices" folder that functions as a secondary labscript_devices folder (using either method you suggested). This makes it easy to prototype devices outside the main repo, though I suppose history then gets lost if mainlining does ultimately occur...

It would also be nice if the secondary folder could also support the 'old-style' look-up, at least temporarily, so I can port over devices to the 'new style' at my leisure. Once everything is ported it should be very easy to subrepo, if desired.

And it's good to know sphinx docs are in the future. I'll be sure to start adding appropriate docs strings.

labscript-suite-temp / labscript_devices

Third-Party Devices #22