Open ispielma opened 7 months ago
@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.
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]
.
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
@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.
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.
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:
At this instant I am doing feature development for
lyse
which usesmodulewatcher.py
. In this caseimp
is being used to get a global import lock, butimportlib
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.