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

Extend argument handling of do_execute with cell metadata #1169

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

jjvraw
Copy link
Contributor

@jjvraw jjvraw commented Nov 20, 2023

Enhances the do_execute method by allowing an optional cell_meta parameter, which represents cell metadata. This addition aligns with the Jupyter messaging protocol on metadata, which acknowledges the potential use of metadata in various message types.

Although metadata may not be often used, the capability to handle metadata within do_execute can expand the kernel's flexibility, especially in scenarios involving complex interactions or custom extensions, as detailed in the jupyter_client docs.

A noteable use case is in the development of Python wrapper kernels, where additional information might be passed via metadata to influence execution behavior, provide additional context, or interact with external services.

Changes:

  • Extending the do_execute signature to include an optional metadata parameter
  • Implementing logic in execute_request to extract and pass down cell metadata to do_execute
    • This involved expanding _accepts_cell_id to _accepts_parameters for broader parameter checks, which in turn seemly created nice flow in execute_request.

It seemed bold requesting to replace cell_id with metadata despite one is derived from the other. However, _accepts_parameters enables flexible handling of these optional parameters, thus somewhat ensuring backward compatibility. A minor concern might involve the imports of _accepts_cell_id, which has been removed/replaced, but can be reintegrated if necessary.

Thanks

@jjvraw jjvraw marked this pull request as draft November 21, 2023 09:10
@jjvraw jjvraw marked this pull request as ready for review November 21, 2023 09:48
@jjvraw
Copy link
Contributor Author

jjvraw commented Nov 21, 2023

Hi,

I've noticed that all checks have passed on the PR except for "Test Prereleases". This seems to be related to an existing issue in pytest-asyncio, which might be causing the failure.

I am happy to work further on any specific actions or suggestions.

Thank you

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! I agree we should keep cell_id, but the private method name can be changed.

@blink1073 blink1073 changed the title extend argument handling of do_execute with cell metadata Extend argument handling of do_execute with cell metadata Nov 21, 2023
@blink1073 blink1073 merged commit 5b72bfe into ipython:main Nov 21, 2023
30 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants