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

ENH: support for PySide6 in %gui #13864

Merged
merged 27 commits into from
Feb 14, 2023
Merged

Conversation

shaperilio
Copy link
Contributor

@shaperilio shaperilio commented Dec 9, 2022

Addresses #13859

Changes here parallel the changes in ipykernel, i.e. prefer PyQt over PySide and allow requesting explicit versions (e.g. 'qt5') with one difference (see below).

I believe that, eventually, the Qt importing logic should be all here, and ipykernel should defer toipython. I chose not to do that at this time since it would mean the latest ipykernel would require the latest IPython; I'm happy to discuss further.

The only difference between IPython and ipykernel, after these two pull requests, is that, in IPython, it's not possible to explicitly request an event loop for Qt4. This is because an alias exists here which effectively makes "qt4" be "the latest Qt available". I did not remove the alias because I don't know the history behind it.

@shaperilio
Copy link
Contributor Author

I'm not sure what to do about the docs failing to build - it doesn't seem to be an issue in main?

@shaperilio
Copy link
Contributor Author

@Carreau what's this force push about?

@Carreau
Copy link
Member

Carreau commented Jan 4, 2023

@Carreau what's this force push about?

Some failures were due to a problem on the main branch, I fixed the test on the main branch and rebase this one top of main, to get the test to pass.

See ipython/ipykernel#1071

Note that it's still there if coming from `matplotlib`
@shaperilio
Copy link
Contributor Author

This is ready for review. The equivalent changes in ipykernel have already been merged.

@shaperilio
Copy link
Contributor Author

There's a problem here with matplotlib, related to commit 5c57ffe.

matplotlib.get_backend() will actually set a backend if none has been set... and then returns it. I think the relevant code is here.

I have an application where I require the TkAgg backend, so I do something like this:

if matplotlib.get_backend() != 'TkAgg':
  _l.debug('IPython shell detected. Attempting to change '
           'matplotlib backend to `TkAgg`...')
  try:
    matplotlib.use('TkAgg')
  except ImportError as e:
    raise RuntimeError('Cannot run. Matplotlib backend cannot be changed to `TkAgg`:\n' 
                       f'   {str(e)}')

Sadly, the call to get_backend actually sets the backend to QtAgg on my system, which hooks in the Qt eventloop. The error I added in 5c57ffe then prevents me from switching to TkAgg, but evidently the exception is getting absorbed somewhere in the call to matplotlib.use.

So I'm changing the error to a warning. My rationale was that I'd want to prevent people from unknowingly changing which event loop was tied in - you could stop some other application from functioning if you do so - but I suppose if you really wanted to prevent that, %gui would have to be changed to support multiple event loops being stacked in the hook.

An exception here causes issues because `matplotlib.get_backend()`
actually sets a backend if none was set.
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @shaperilio for your work on this! I left some comments for you, which are basically the things I changed in ipykernel to provide a better user experience, in my opinion.

IPython/terminal/pt_inputhooks/__init__.py Outdated Show resolved Hide resolved
IPython/terminal/pt_inputhooks/__init__.py Outdated Show resolved Hide resolved
@shaperilio
Copy link
Contributor Author

Thanks @shaperilio for your work on this! I left some comments for you, which are basically the things I changed in ipykernel to provide a better user experience, in my opinion.

Thanks for these. I should have learned my lesson with the matplotlib issue. :)

@shaperilio shaperilio changed the title ENH: support for PySide6 in %gui` ENH: support for PySide6 in %gui Feb 1, 2023
@shaperilio
Copy link
Contributor Author

Any thoughts on when this may be merged, or if there's more feedback? I believe it's been released in ipykernel.

@Carreau
Copy link
Member

Carreau commented Feb 14, 2023

Sorry, this fell under the radar. Let's try it.

@Carreau Carreau merged commit 0374cf8 into ipython:main Feb 14, 2023
@Carreau Carreau added this to the 8.11 milestone Feb 14, 2023
Carreau added a commit that referenced this pull request Mar 13, 2023
I started using the released version of my `PySide6`-enabling changes
and noted some problems. In this PR, I fix those, and also overall
improve the feedback to the user when a GUI event loop is hooked in:

- Report which event loop is running when using `%gui <some GUI>`; e.g.
`%gui qt` will show `Installed qt6 event loop hook.`
- Report when the event loop is disabled; i.e. `%gui` will show `GUI
event loop hook disabled.` if an event loop hook was installed, or `No
event loop hook running.` if nothing was installed.
- Requesting a second event loop will give the message `Shell is already
running a gui event loop for <some GUI>. Call with no arguments to
disable current loop.`
- Requesting a different version of Qt, i.e. `%gui qt6` followed by
`%gui` followed by `%gui qt5` will show `Cannot switch Qt versions for
this session; will use qt6.` followed by `Installed qt6 event loop
hook.`

(Fixes / improves #13864)
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.

None yet

3 participants