imp module removed from new python

[ispielma]

the imp module has been depreciated since 3.4 and is now removed from python. importlib is the replacement.

I found it being used in the following:

labscript_utils/device_registry/_device_registry.py
labscript_utils/double_import_denier.py
labscript_utils/modulewatcher.py

At this instant I am doing feature development for lyse which uses modulewatcher.py. In this case imp is being used to get a global import lock, but importlib does not support this functionality.

I am looking into this right now, but this is a hight priority issue because it blocks labscript from being used in current python releases.

[dihm]

#101 patches this for the time being, but we should really look into module watcher and see if we can simplify it. Checking the deprecation guide , the primary hurdle is import locks, which have fundamentally changed so much that a global import lock just isn't provided any more. There is a decent chance that the move to module-level import locks has removed the need for locks, but we would want to confirm that before stripping things out.

[dihm]

@ispielma Something has changed here and the change in #101 is now breaking for me on BLACS during device registry discovery.

Traceback (most recent call last):
  File "C:\Users\naqsL\miniconda3\envs\labscript\Lib\threading.py", line 982, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\naqsL\miniconda3\envs\labscript\Lib\site-packages\zprocess\utils.py", line 122, in _reraise
    raise value.with_traceback(traceback)
  File "C:\Users\naqsL\src\labscript-suite\blacs\blacs\__main__.py", line 271, in __init__
    TabClass = device_registry.get_BLACS_tab(labscript_device_class_name)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\naqsL\src\labscript-suite\labscript-utils\labscript_utils\device_registry\_device_registry.py", line 215, in get_BLACS_tab
    populate_registry()
  File "C:\Users\naqsL\src\labscript-suite\labscript-utils\labscript_utils\device_registry\_device_registry.py", line 269, in populate_registry
    fp, pathname, desc = imp.find_module('register_classes', [folder])
                         ^^^^^^^^^^^^^^^
AttributeError: module '_imp' has no attribute 'find_module'

I appear to get this error for all versions of python that I've tested with on one of my dev computers [3.9.18, 3.12.2, 3.11.7].
A potential hotfix that makes the break less common is to swap the try/except block so that it defaults to old behavior for most versions of python.

try:
    import imp
except ImportError:
    import _imp as imp

Ultimately, it appears that find_module is not actually part of the _imp package for any python version I can find. I suspect that modernizing this functionality needs to become a much higher priority.

[ispielma]

Doh! So my initial hotfix worked for one case but broke another. I will look into this PM today. Do you offhand know any other place in Labscript where imp is used so I can check for compatibility? The suggested replacement for find_module is importlib.util.find_spec(name, package=None (https://docs.python.org/3.11/library/importlib.html#importlib.util.find_spec), but it is unclear if the package= kwarg is equivalent to [folder].

[dihm]

As far as I can tell, only the three parts of labscript_utils you mention at the top use imp.

May not be helpful, but I did find this interesting thread related to this, including this useful snippet for replacing find_module and load_module that may be close to drop-in replacement of the populate registry code.

spec = importlib.machinery.PathFinder.find_spec(modname, [""])
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
sys.modules[modname] = mod

[dihm]

@ispielma I think I have a functional pass at updating the device registry importing

def populate_registry():
    """Walk the labscript_devices folder looking for files called register_classes.py,
    and run them (i.e. import them). These files are expected to make calls to
    register_classes() to inform us of what BLACS tabs and runviewer classes correspond
    to their labscript device classes."""
    # We import the register_classes modules as a direct submodule of labscript_devices.
    # But they cannot all have the same name, so we import them as
    # labscript_devices._register_classes_<num> with increasing number.
    module_num = 0
    for devices_dir in LABSCRIPT_DEVICES_DIRS:
        for folder, _, filenames in os.walk(devices_dir):
            if 'register_classes.py' in filenames:
                # The module name is the path to the file, relative to the labscript suite
                # install directory:
                # Open the file using the import machinery,
                # and import it as labscript_devices._register_classes_<num>.
                spec = importlib.machinery.PathFinder.find_spec('register_classes', [folder])
                # spec and loader must have the same updated name
                spec.name = 'labscript_devices._register_classes_%d' % module_num
                spec.loader.name = spec.name
                mod = importlib.util.module_from_spec(spec)
                spec.loader.exec_module(mod)
                sys.modules[spec.name] = mod
                module_num += 1

Given how python has changed since this code was first written, I actually wonder if the module even needs to be added to the imported modules list, since we only need it to execute once (to do the registering). May be able to simplify this a bit by dropping sys.modules[spec.name] = mod. If we can do that, we don't even need to mux with the name, we can just leave things at

spec = importlib.machinery.PathFinder.find_spec('register_classes', [folder])
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)

This simplification would require more thorough testing to ensure edge cases didn't break.

[ispielma]

Issues might arise in the restart tab option which might (or might not since I am not looking at it right now) grab the module reference from sys.modules rather than re-searching.

....

It looks like this is not an issue. BLACS tabs have to define a restart method and the processes seem not to be actually restarted.