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.

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

Manifest bump ipython to new version will fail the test #6697

Closed
wants to merge 3 commits into from

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Jan 10, 2025

uv.lock is updated by running uv lock --upgrade-package ipython. The problem was found and redirected from #6694

@unkcpz unkcpz requested a review from agoscinski as a code owner January 10, 2025 14:23
@unkcpz
Copy link
Member Author

unkcpz commented Jan 10, 2025

The problem can not manifest by updating the ipython, the failed test is:

     def test_ipython_magics():
        """Test that the ``%aiida`` magic can be loaded and imports the ``QueryBuilder`` and ``Node`` classes."""
>       ipy = get_ipython()

tests/tools/ipython/test_ipython_magics.py:12: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.venv/lib/python3.12/site-packages/IPython/testing/globalipapp.py:27: in get_ipython
    return start_ipython()
.venv/lib/python3.12/site-packages/IPython/testing/globalipapp.py:69: in start_ipython
    shell = TerminalInteractiveShell.instance(config=config,
.venv/lib/python3.12/site-packages/traitlets/config/configurable.py:583: in instance
    inst = cls(*args, **kwargs)
.venv/lib/python3.12/site-packages/IPython/terminal/interactiveshell.py:853: in __init__
    super(TerminalInteractiveShell, self).__init__(*args, **kwargs)
.venv/lib/python3.12/site-packages/IPython/core/interactiveshell.py:601: in __init__
    self.init_virtualenv()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <IPython.terminal.interactiveshell.TerminalInteractiveShell object at 0x7fa35657af60>

    def init_virtualenv(self):
        """Add the current virtualenv to sys.path so the user can import modules from it.
        This isn't perfect: it doesn't use the Python interpreter with which the
        virtualenv was built, and it ignores the --no-site-packages option. A
        warning will appear suggesting the user installs IPython in the
        virtualenv, but for many cases, it probably works well enough.
    
        Adapted from code snippets online.
    
        http://blog.ufsoft.org/2009/1/29/ipython-and-virtualenv
        """
        if 'VIRTUAL_ENV' not in os.environ:
            # Not in a virtualenv
            return
        elif os.environ["VIRTUAL_ENV"] == "":
            warn("Virtual env path set to '', please check if this is intended.")
            return
    
        p = Path(sys.executable)
        p_venv = Path(os.environ["VIRTUAL_ENV"])
    
        # fallback venv detection:
        # stdlib venv may symlink sys.executable, so we can't use realpath.
        # but others can symlink *to* the venv Python, so we can't just use sys.executable.
        # So we just check every item in the symlink tree (generally <= 3)
        paths = self.get_path_links(p)
    
        # In Cygwin paths like "c:\..." and '\cygdrive\c\...' are possible
>       if p_venv.parts[1] == "cygdrive":
E       IndexError: tuple index out of range

.venv/lib/python3.12/site-packages/IPython/core/interactiveshell.py:890: IndexError

@unkcpz
Copy link
Member Author

unkcpz commented Jan 10, 2025

I track to ipython/ipython#13268, and find it relates to the VIRTUAL_ENV which @danielhollas you may adapted a bit in ci? Any idea?

Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.99%. Comparing base (5e8bbe1) to head (a832fd6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6697   +/-   ##
=======================================
  Coverage   77.99%   77.99%           
=======================================
  Files         563      563           
  Lines       41761    41761           
=======================================
  Hits        32567    32567           
  Misses       9194     9194           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@unkcpz unkcpz marked this pull request as draft January 10, 2025 15:03
@danielhollas
Copy link
Collaborator

We already have a workaround for this that's only applied for python 3.9, and related to setup-uv action.

I suspect you're triggering this even for Python. 3.12 because you have an old uv version locally. I will need to think about if / how to enforce minimum uv version.

I think this PR is not needed, we're just waiting for a new setup-uv release and then we'll be able to drop the existing workaround.

@unkcpz unkcpz force-pushed the bump-ipytho-fail-test branch from 1835258 to 7e741d4 Compare January 10, 2025 15:26
@unkcpz
Copy link
Member Author

unkcpz commented Jan 10, 2025

As you mentioned, it is the problem with my local uv version. Thanks for help.

@unkcpz unkcpz closed this Jan 10, 2025
@unkcpz unkcpz deleted the bump-ipytho-fail-test branch January 10, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants