Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix add_to_spyder_UMR_excludelist for Spyder 6 #6544

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix add_to_spyder_UMR_excludelist for Spyder 6 #6544

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4580a13
Fix add_to_spyder_UMR_excludelist for Spyder 6
sldesnoo-Delft Oct 18, 2024
5efe1e4
Added check if running in IPython and magic installed by Spyder
sldesnoo-Delft Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 36 additions & 27 deletions src/qcodes/utils/spyder_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import logging
import os
from packaging.version import Version


_LOG = logging.getLogger(__name__)

Expand All @@ -11,39 +13,46 @@
store global attributes such as default station, monitor and list of
instruments. This "feature" can be disabled by the
gui. Unfortunately this cannot be disabled in a natural way
programmatically so in this hack we replace the global ``__umr__`` instance
with a new one containing the module we want to exclude. This will do
programmatically so in this hack we retrieve the UMR instance
and add the module we want to exclude. This will do
nothing if Spyder is not found.
TODO is there a better way to detect if we are in spyder?
"""
if any("SPYDER" in name for name in os.environ):
sitecustomize_found = False
try:
from spyder.utils.site import ( # pyright: ignore[reportMissingImports]
sitecustomize,
)
except ImportError:
pass
else:
sitecustomize_found = True
if sitecustomize_found is False:
try:
from spyder_kernels.customize import spydercustomize # pyright: ignore
import spyder_kernels # pyright: ignore

Check warning on line 23 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L23 

Added line #L23 was not covered by tests

sitecustomize = spydercustomize
except ImportError:
pass
if Version(spyder_kernels.__version__) < Version("3.0.0"):

Check warning on line 25 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L25 

Added line #L25 was not covered by tests
# In Spyder 4 and 5 UMR is a variable in module spydercustomize
try:
from spyder_kernels.customize import spydercustomize # pyright: ignore
except ImportError:
return

Check warning on line 30 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L27-L30 

Added lines #L27 - L30 were not covered by tests
else:
umr = spydercustomize.__umr__

Check warning on line 32 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L32 

Added line #L32 was not covered by tests
else:
sitecustomize_found = True
# In Spyder 6 UMR is an attribute of the SpyderCodeRunner object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also encountered qcodes not running with the new Spyder 6. The UserModuleReloader is also available via a normal import:

import spyder_kernels.customize.umr; spyder_kernels.customize.umr.UserModuleReloader

Using that instead of sitecustomize.UserModuleReloader works for me so far.

@ccordoba12 Do you have an opinion on how to approach this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that as well.
Unfortunately Spyder6 does not use the global umr. So assignment of the changed UMR has no effect.

That's why I searched for the UMR instance that is being used and modify that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eendebakpt @sldesnoo-Delft I am not using spyder my self at the moment so unlikely to spot this.

At some point a long time ago I did look into addressing this issue on the spyder side spyder-ide/spyder#2451 If anyone have bandwidth to do that it would be great as this is a much cleaner option.

Otherwise happy to merge this once you have converted on the best solution

Copy link

@ccordoba12 ccordoba12 Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccordoba12 Do you have an opinion on how to approach this?

I wasn't aware people was adding modules to our UMR programmatically (no one has reported it to our issue tracker).

Since the code to do that for Spyder 6/Spyder-kernels 3 is too convoluted (from what I can see below), we could add a public API at the shell level (i.e. the object you get with get_python()) to make the life of other projects easier.

@jenshnielsen, @sldesnoo-Delft, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qcodes adds itselft to the list of packages to exclude.

I had a closer look at the UMR. It checks if the module is in the library path. (path_is_library in utils.py). When qcodes is normally installed, i.e. not in "develop mode", then it will not be reloaded by UMR.

So, I believe the add_to_spyder_UMR_excludelist function is not really needed. It might be a remainder from a distant past.

@jenshnielsen , @eendebakpt do you agree?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sldesnoo-Delft I think this has ever only been a problem with editable installs. As I recall the problem is that when spyder does reload all singletons are destroyed so instruments are lost. I think that when we originally added this qcodes did not see regular releases, so a lot of users were installing from source. This is much less of an issue now so I think it would be fine to deprecate/remove this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no strong opinion on this, we usually disable the spyder UMR (other packages have issues with automatic reloading as well). We only noticed because the new spyder 6 would not import qcodes.

If we do remove the functionality, I do think it warrants some attention in the release notes (and maybe the documentation?).

# This object can be found via the magics manager
from IPython import get_ipython # pyright: ignore
ipython = get_ipython()
if ipython is None:

Check warning on line 38 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L36-L38 

Added lines #L36 - L38 were not covered by tests
# no ipython environment
return
try:
runfile_method = ipython.magics_manager.magics['line']['runfile']
except KeyError:

Check warning on line 43 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L40-L43 

Added lines #L40 - L43 were not covered by tests
# no runfile magic
return
spyder_code_runner = runfile_method.__self__
umr = spyder_code_runner.umr

Check warning on line 47 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L45-L47 

Added lines #L45 - L47 were not covered by tests

if sitecustomize_found is False:
return
excludednamelist = os.environ.get("SPY_UMR_NAMELIST", "").split(",")
if modulename not in excludednamelist:
_LOG.info(f"adding {modulename} to excluded modules")
excludednamelist.append(modulename)
if modulename not in umr.namelist:
umr.namelist.append(modulename)
os.environ["SPY_UMR_NAMELIST"] = ",".join(excludednamelist)
except Exception as ex:
_LOG.warning(f"Failed to add {modulename} to UMR exclude list. {type(ex)}: {ex}")

Check warning on line 57 in src/qcodes/utils/spyder_utils.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/utils/spyder_utils.py#L49-L57 

Added lines #L49 - L57 were not covered by tests

excludednamelist = os.environ.get("SPY_UMR_NAMELIST", "").split(",")
if modulename not in excludednamelist:
_LOG.info(f"adding {modulename} to excluded modules")
excludednamelist.append(modulename)
sitecustomize.__umr__ = sitecustomize.UserModuleReloader( # pyright: ignore[reportPossiblyUnboundVariable]
namelist=excludednamelist
)
os.environ["SPY_UMR_NAMELIST"] = ",".join(excludednamelist)
Loading