labscript-suite-temp / labscript_utils

Shared modules used by the labscript suite. Includes a graphical exception handler, debug tools, configuration management, cross platform filepath conversions, unit conversions and custom GUI widgets.
Other
0 stars 0 forks source link

Double import denier breaks with runviewer v2.1.0 #18

Closed philipstarkey closed 6 years ago

philipstarkey commented 6 years ago

Original report (archived issue) by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).


The latest tagged version of runviewer is v2.1.0. When using the tagged (or tip) version of labscript utils, the following exception is raised:

#!python

Traceback (most recent call last):
  File "C:\labscript_suite\labscript_py27\runviewer\__main__.py", line 78, in <module>
    from resample import resample as _resample
RuntimeError: Double import! The same file has been imported under two different names, resulting in two copies of the module. This is almost certainly a mistake. If you are running a script from within a package and want to import another submodule of that package, import it by its full path: 'import module.submodule' instead of just 'import submodule.'

Path imported: C:\labscript_suite\labscript_py27\runviewer\resample

Traceback (first time imported, as resample):
------------
  File "C:\labscript_suite\labscript_py27\runviewer\__main__.py", line 78, in <module>
    from resample import resample as _resample
------------

Traceback (second time imported, as runviewer.resample):
------------
  File "C:\labscript_suite\labscript_py27\runviewer\__main__.py", line 78, in <module>
    from resample import resample as _resample
  File "C:\labscript_suite\labscript_py27\runviewer\resample\__init__.py", line 33, in <module>
    module = importlib.import_module('runviewer.resample.%s.resample'%plat_name)
  File "C:\Anaconda3\envs\labscript_py27\lib\importlib\__init__.py", line 37, in import_module
    __import__(name)
------------

Updating runviewer to tip fixes the issue (and so we should really tag a new runviewer version ASAP), but only because we change the way we compile and use the resample algorithm. However it looks to me like the double import denier is still being too aggressive here because a single import line triggered the double import denier.

philipstarkey commented 6 years ago

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


I don't think it's too aggressive, I'd argue that this is the exact issue that the double import denier was designed to catch.

Even though it's a single import line that triggers it, the imported module goes on to imports itself under a different name. Basically by importing runviewer.resample.%s.resample, it is first importing runviewer.resample, i.e. itself, under a different name to what the calling code imported it as. So this would have led to two copies of itself in memory if not caught (and so runviewer has probably had a double import all this time).

In this case probably harmless, but I think correctly identified as a double import.

There have been a few issues with double import denier though, and I am considering that maybe it shouldn't be turned on for all code importing labscript_utils, and perhaps it should be explicitly imported and enabled by the GUI programs only. Keeping it away from arbitrary user code that might be doing other funky things with the import system might be a good idea. But I might wait to see if any more issues turn up before changing that, since maybe it's sufficiently bug-free now.

You want to change the __version__ attribute and tag a version bump? Either minor or patch number, I'm not sure how much has changed since the last tagged version.

philipstarkey commented 6 years ago

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


Ok.

runviewer released as 2.2.0 as of 4cc49b8d35fb542e11fb2ff039ba7f9bb437a51a.