labscript-suite-temp-2 / labscript

The labscript Python library provides a translation from simple Python code to complex hardware instructions. The library is used to construct a "connection table" containing information about what hardware is being used and how it is interconnected. Devices described in this connection table can then have their outputs set by using a range of functions, including arbitrary ramps.
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Globals and devices should be in modules, not builtins: `shot_globals` and `shot_devices` #11

Open philipstarkey opened 10 years ago

philipstarkey commented 10 years ago

Original report (archived issue) by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).


I used to not think it mattered much that we were modifying builtins to put our globals in them. Mainly because, although completely sacrilegious, we have a sort-of domain specific language here, damnit, we should be able to do what we like.

But more pragmatically I figured it wouldn't affect anything. Our code is the toplevel code, and nothing is relying on it conforming to good practices. Code we use will be unaffected because scoping rules.

This is not true. There is correct, unbuggy code out there that relies on getting NameErrors on things:

#!bash

bilbo:~ $ grep -R 'except NameError' /usr/lib/python2.7/ | wc -l
301

often for telling the difference between different versions of Python, but also for other totally innocent reasons.

But also I acknowledge that deviating from accepted practices confuses people, even when it doesn't to do any harm. This inhibits their ability to make good assumptions about how your software behaves.

So all the code that puts stuff into builtins should instead put them in two programmatically created modules, called shot_globals and shot_devices. These will be inserted into sys.modules so the user can import them from anywhere.

The user will probably want to import * from both (but they don't have to), and either way it makes it clearer in there code where stuff can be coming from, and no code that doesn't want to see our crap has to.

The only other question is: do we still make device instantiation put device objects into the global namespace they're being defined in? This is important because the user will often be immediately using the device they just defined as the parent of subsequently defined child devices.

I think we should not do this. The user should define their connection table and then do from shot_devices import * (or whatever they want to import).

To handle the fact that devices are needed within the connection table, we should change it such that parent devices are specified with strings instead of python identidiers from the current namespace.

Any other methods the user wants to call on devices halfway through the connection table should generally wait until the end, but if necessary, the user can import shot_devices as is, or import specific devices from it at any point. So long as they save their import * until the end. I can add an import hook that will take note of import * for the shot_devices module, such that Device.__init__ can raise an exception if a new device is being added after an import * was done. Using the same introspection that I'll be using to give labscript users better errors about compilation, I'll even be able to store the traceback to the import * call. No more of this `error: something was imported once, I don't know when, but don't do that!' like h5_lock and lyse's figure manager do (both of which should be changed to turn on their functionality on a function call, rather than as an import side effect).

Discussion welcome.

philipstarkey commented 10 years ago

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


I'm a little against not putting devices in the namespace, because I think specifying parent devices as strings is going to be messy. It isn't necessarily just the device name that is the parent any more. In the gated-clocks branch, a DDS attached to a PulseBlaster now has parent .direct_outputs (for example).

If we move to strings, then we effectively enforce a convention for how to refer to internally created child devices. Right now, someone could set the parent of a child device as .get_clockline(7) or other complex I can't see an easy way to maintain that freedom with strings that is both intuitive and not doing something like exec'ing a string.

Not sure if there is an ideal solution...

philipstarkey commented 10 years ago

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


Oh, I hadn't thought of that. That's a decent objection, I wonder if there's any other non-magical way to give the user access to the devices themselves, rather than using strings, without putting them in the namespace, or anything else like that. Hm. I meant they can always do import shot_devices as d and do d.<parent_device>.get_clockline(7), but then they'll probably want to do from shot_devices import * too later, and it is getting pretty bizarre.

We could demand they assign their devices to names pulseblaster_1 = PulseBlaster('pulseblaster_1')..., but that is awful too.

Yeah, I think putting them in the namespace is looking like the best solution.

It will actually have to be the global namespace, even if the user is defining their connection table in a function (as I suggest in issue #10 they should do if the connection table is not in their main file). It can't be put in the function namespace because in Python you actually cannot modify an enclosing namespace, only local or global ones (in Python 2, anyway, unsure whether this changed with the introduction of the nonlocal keyword in Python 3).

For consistency should we make shot globals global to the main labscript file too? They are called 'globals', after all. And then we're just telling people that if they need shot globals outside the main file, they can import them, and if they need devices outside the module that defined them, they can import them.

philipstarkey commented 10 years ago

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


I've been wondering whether we should have the user explicitly call labscript_init(h5_file) in labscript code, and have the only magical thing runmanger does be inserting that h5_file varialble into the namespace, kinda like how lyse does it.

This would make it once again symmetric between compiling shots standalone and from runmanager, and would define which module gets the globals: instantiating a device puts it into your global namespace, calling labscript_init() puts globals in your global namespace. I'll report this as another issue, it has advantages of its own.