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

Unexpected results when importing connection table #10

Open philipstarkey opened 9 years ago

philipstarkey commented 9 years ago

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


If a user offloads their connection table to a separate module and imports it from their main experiment script, it works fine, at first.

The second time they click 'engage' in runmanager, however, they will find that they have no devices defined.

This is because the module was imported, and their code is running in the same interpreter process as before, and so the import statement just returns a cached module. The module has not changed, so ModuleWatcher does not reload it. This would be fine if the connection table defined objects to be referred to, but it doesn't - those Device.__init__ methods are special, and it is important that they actually run again each time.

I'm not totally sure what to do about this. One solution is documentation:

"Dear user, these may look like object definitions, but they are actually procedural. You need this code to actually run, not just be imported, and the fact that code runs when imported is actually bad practise and shouldn't be relied upon. It only does so the first time, and your module can never know how many times it is going to be expected to run its code. Please wrap your connection table in a function called something like define_connection_table(), and call it, rather than relying on the fact that module-level code gets run on the first import"

The other solutions are likely the darkest black magic, and would make Guido cry. I don't want to make Guido cry, and so would only really consider other solutions if they were sufficiently nonmagical.

Much easier to tolerate though, is the sort of magic required to detect when a Device class is being instantiated at the toplevel of anything but the main labscript file, and shoot the above warning at the user. In fact, not a warning, an exception probably.

Yes, I think that is the correct solution, but I need to think about how it might interact with another issue I'm about to create, in which I'll document our previous decision that stuff we're currently inserting into builtins should actually be placed in a dynamically generated module for user import. Because regardless of the resolution of this issue, the builtins hammering would certainly have the BDFL in tears.

philipstarkey commented 9 years ago

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


philipstarkey commented 9 years ago

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


Although it's poor form to rely on import side effects, everybody does it (labscript stuff is especially guilty of this), so we should probably cater to it, especially since I realised how.

Instead of using magic to tell the user not to instantiate Devices in the top level of modules, we should use the same sort of magic to note these modules and delete them from sys.modules between compilations (like ModuleWatcher does) so they always get reloaded. To be totally bomb-proof we will also have to delete any modules that import these modules, and so on. There is an import tracer in labscript_utils that I can retool for this purpose - this functionality probably belongs in the ModuleWatcher module.