-
Notifications
You must be signed in to change notification settings - Fork 61
#329 add on cell input request #333
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
base: main
Are you sure you want to change the base?
Conversation
Would you like that as part of this PR or a separate one?
~ Dan Grahn, Ph.D.
Pardon my brevity. I sent this from my phone.
…On Fri, Jul 18, 2025, 12:57 PM David Brochart ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nbclient/client.py
<#333 (comment)>:
> @@ -782,7 +782,7 @@ async def _async_poll_stdin_msg(
while True:
try:
- msg = await ensure_async(self.kc.stdin_channel.get_msg(timeout=None))
+ msg = await ensure_async(self.kc.stdin_channel.get_msg(timeout=self.iopub_timeout))
Why not having timeout=None and getting rid of the try/except?
—
Reply to this email directly, view it on GitHub
<#333 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADALVL64XMADH5AFHUFS6L3JERRFAVCNFSM6AAAAACBXXO6WSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAMZUGIYTIMRUGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
What are you referring to? |
Sorry. That was mean to be in reply to the conversation about |
As you wish, I think it's fine to do it in this PR. |
I'll submit another PR changing all of them. |
The switch from |
Hi @davidbrochart. Are you and the team planning to merge this? I'm trying to decide whether to fork and run with the project there or continue contributing. |
This PR is in response to #329. It adds a handler which allows the user of the NotebookClient to respond to input requests. This feature is lacking in many tools that run Jupyter Notebooks via script. Hopefully this will help facilitate the functionality in other places. I personally plan to use this feature very soon from the repo directly and will upgrade to pip as soon as it is available.
The implementation was fairly straightforward, but slightly more complicated than the original issue to blend more seamlessly. There is now a new async polling task that watches for input requests and then hands them off to the handler. The handler receives
cell
,cell_index
, andinput_request
. If the handler is not set, this task is never spawned and functionality remains as is.Tests were also modified to ensure that this new functionality was covered.
Usage
Detailed Changes
on_cell_input_request
hook to theNotebookClient
class, allowing users to handle input requests during cell execution. The hook is called with argumentscell
,cell_index
, andinput_request
. (nbclient/client.py
, nbclient/client.pyR328-R338)allow_stdin
only if theon_cell_input_request
hook is defined. (nbclient/client.py
, nbclient/client.pyL575-R586)_async_poll_stdin_msg
, an asynchronous method to handle input requests from the kernel by invoking theon_cell_input_request
hook and sending the response back to the kernel. (nbclient/client.py
, nbclient/client.pyR772-R796)async_execute_cell
method, ensuring input polling tasks are created, managed, and properly canceled. (nbclient/client.py
, Fae205e2L996R1050)InputRequest.ipynb
) containing a cell with aninput()
call to validate the input request functionality. (tests/files/InputRequest.ipynb
, tests/files/InputRequest.ipynbR1-R21)on_cell_input_request
hook and mock its behavior for various test cases. (tests/test_client.py
, [1] [2]test_input_request_hook
) to verify theon_cell_input_request
hook is called correctly and with the expected arguments when a cell requests input. (tests/test_client.py
, tests/test_client.pyR974-R1005)Fixes: #329