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

Create event / signal at start of cell execution #118

Closed
psychemedia opened this issue Apr 19, 2024 · 4 comments
Closed

Create event / signal at start of cell execution #118

psychemedia opened this issue Apr 19, 2024 · 4 comments

Comments

@psychemedia
Copy link

The NotebookActions.executionScheduled and NotebookActions.executed events allow us to identify when a ntoebook cell is queued for execution and when it completes, but there is no signal for when it starts execution.

Would it be possible for this extension to generate a signal that other extensions can connect to that identifies when a cell starts executing?

@krassowski
Copy link
Collaborator

Would it be possible for this extension to generate a signal that other extensions can connect to that identifies when a cell starts executing?

Currently this extension does not emit signals and has no public API. On implementation level, all it does is listen to cell metadata changes (using CellModel.metadataChanged signal) and reflects the status accordingly. In particular to know if execution started it checks for shell.execute_reply.started metadata in execution stanza. While it would be possible to add a signal as you propose, I think that creating a signal for other extensions to use should not be a responsibility of this extension as all it would be just proxying changes which are otherwise directly available in cell model.

@psychemedia
Copy link
Author

psychemedia commented Apr 19, 2024

Okay - thanks... just feels a bit redundant for multiple extensions to have to set up their own listeners on the metadata. (I assume core JL maintainers have a reason not to want to offer the event as a core signal.)

@krassowski
Copy link
Collaborator

I assume core JL maintainers have a reason not to want to offer the event as a core signal

I don't think so. In fact when executionScheduled was being added I suggested to also add executionStarted too (jupyterlab/jupyterlab#10259 (comment)) but the contributor who did spend their time writing the code did not need it so they did not include it in jupyterlab/jupyterlab#10493:

Note: @krassowski I didn't add the executionStarted signal because it didn't seem necessary. The started time had no noticeable difference compared to executionScheduled's start time.

... which I don't think is the case in general. Anyways, if you want to open a PR to core I would be happy to review it :)

@psychemedia
Copy link
Author

Ah, interesting history, thanks. Agree with the "not generally true", eg if you Run All etc.

Re: a PR, I'm not sure my coding, or understanding of how JL works under the hood, is up to it.

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

No branches or pull requests

2 participants