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

Correctly handle with_cell_id in async do_execute #1227

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

ianthomas23
Copy link
Collaborator

This fixes the minimum dependency tests following the switch to AnyIO (#1079).

In IPythonKernel.do_execute we determine if cell_id is supported using

with_cell_id = _accepts_parameters(run_cell, ["cell_id"])

or similar. This returns a dict, either {"cell_id", True} or {"cell_id", False}.

But when testing this we use

if with_cell_id:

which gives the correct answer if cell_id is supported but the wrong one otherwise. It needs to be

if with_cell_id and with_cell_id["cell_id"]:

instead, which is all this PR does.

It happened to work for IPython >= 8.3.0 because of ipython/ipython#13600 so that cell_id is always accepted, but failed for IPython <= 8.2.0.

@ianthomas23 ianthomas23 added the bug label Apr 3, 2024
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thank you!

@blink1073 blink1073 merged commit 8d60e67 into ipython:main Apr 3, 2024
23 of 32 checks passed
@ianthomas23 ianthomas23 deleted the fix_with_cell_id branch April 3, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants