Skip to content

Fix inputhook integration - Pass inputhook as an argument to Promptsesion.run().#14241

Merged
Carreau merged 1 commit into
ipython:mainfrom
jonathanslenders:fix-eventloop-inputhook
Nov 20, 2023
Merged

Fix inputhook integration - Pass inputhook as an argument to Promptsesion.run().#14241
Carreau merged 1 commit into
ipython:mainfrom
jonathanslenders:fix-eventloop-inputhook

Conversation

@jonathanslenders
Copy link
Copy Markdown
Contributor

See: prompt-toolkit/python-prompt-toolkit#1809

To summarize:

  • In prompt_toolkit, some changes were done to get rid of a deprecation warning related to the usage of set_event_loop() on Python 3.12. It looks like IPython did had a workaround for the deprecation warning, but Xonsh for instance did not. Now, the code that was fixed in prompt_toolkit breaks input hooks in IPython.
  • The right thing to do would be to accept the input hook as an argument in prompt_toolkit, which is what this PR does: Fix inputhook implementation to be compatible with asyncio.run(). prompt-toolkit/python-prompt-toolkit#1810
  • For IPython, we have to do a similar change to pass it as an argument (this PR).

It's not well tested (I'm not using IPython with inputhooks myself). I'm also not very familiar myself with IPython's code base.

I think it requires some coordination. I'm not sure if there's a way to be backward compatible. I guess if I push a new prompt_toolkit version with the fix on my side, nothing will break, but input hooks will for sure not work on any Python version until this is merged.

We should probably set the minimum prompt_toolkit version after merging this.

@Carreau Carreau added this to the 8.18 milestone Nov 13, 2023
@Carreau
Copy link
Copy Markdown
Member

Carreau commented Nov 13, 2023

I you know that you will update the code in prompt toolkit for 3.0.41, then I can help you rewrite this with a conditional on the prompt toolkit version.

If it's older prompt_toolkit and python 3.12, we can print a warning.

This way I can release IPython 8.18 at the end of the month (or earlier if you wish).
And then you can do a release of 3.0.41. I guess we can also maybe use inspect signature to see if you take parameters ?

@jonathanslenders
Copy link
Copy Markdown
Contributor Author

@Carreau: Thank you! To be clear, prompt_toolkit==3.0.40, which was released last Friday is breaking IPython inputhooks right now.

I merged prompt-toolkit/python-prompt-toolkit#1810 . I can already release 3.0.41, this won't make a difference, except for Python <3.10 where inputhooks currently still work, those will be broken as well, assuming inputhooks need to be passed as a parameter.

@jonathanslenders
Copy link
Copy Markdown
Contributor Author

@Carreau : I merged a workaround in prompt_toolkit, which detects whether it's called from IPython and no inputhook argument is being passed: prompt-toolkit/python-prompt-toolkit#1811 This fixes the %gui integrations again. It's released as 3.0.41
It would still be good however to proceed with the changes over here.

@Carreau Carreau changed the title [wip] Fix inputhook integration - Pass inputhook as an argument to Promptsesion.run(). Fix inputhook integration - Pass inputhook as an argument to Promptsesion.run(). Nov 20, 2023
@Carreau
Copy link
Copy Markdown
Member

Carreau commented Nov 20, 2023

Ok, let's get that at least in the main branch. I'll do a release on Friday.

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