Add execution timings in code cell metadata for v4 spec

[Madhu94]

Ref: jupyterlab/jupyterlab#5009

[saulshanabrook]

Thanks for submitting this @Madhu94! We want to start being able to record timing information in JupyterLab, so if anyone has feedback on this proposal that would be helpful.

[westurner]

Sorry, I haven't yet read the proposal. Is this optional? Or will every nb diff have timing noise in it? TBH, defaulting to off may be best solely to preserve diffs of runs on different machines

…

On Wednesday, August 7, 2019, Saul Shanabrook ***@***.***> wrote: Thanks for submitting this @Madhu94 <https://github.com/Madhu94>! We want to start being able to record timing information in JupyterLab, so if anyone has feedback on this proposal that would be helpful. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#144?email_source=notifications&email_token=AAAMNSZ76T4EGRH4X6LAOKDQDLX7TA5CNFSM4IIKQ6N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3Y5QKY#issuecomment-519165995>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAMNS6DJHJCRRAFBXYBXALQDLX7TANCNFSM4IIKQ6NQ> .

[saulshanabrook]

Yeah it should be optional.

In JupyterLab the proposal is to default to not storing timing.

[minrk]

Makes sense. It's probably worth noting in the spec that this should be an opt-in behavior that should be used with great caution when working with version control. The PR also specifies the fields and meaning of several timings. Do you want to include those in the spec here?

[Madhu94]

@minrk The schema was somewhat loose and doesn't refer to the timings directly. Should I add those ?

[saulshanabrook]

@Madhu94 You could add them as optional and also allow other unregistered options. I had a first go at some descriptions already:

• Starting:
  • timing.iopub.status.busy: header.date of iopub status busy message
  • timing.iopub.execute_input: header.date of iopub execute_input message
  • timing.shell.execute_reply.started: metadata.started of shell execute_reply message
• Ending
  • timing.shell.execute_reply: header.date of shell execute_repl
  • timing.iopub.status.idle: header.date of iopub status idle message

[Madhu94]

Minor nitpick - do you think execute_reply.started should belong in nbformat? It isn't really part of the official jupyter spec and comes with the metadata from the ipykernel, so other kernels may not have it.

Edit: The comment there also says started is part of ipyparallel and it would be removed.

[saulshanabrook]

do you think execute_reply.started should belong in nbformat? It isn't really part of the official jupyter spec and comes with the metadata from the ipykernel, so other kernels may not have it.

Good catch! Yeah then I wouldn't include it, thanks.

[Madhu94]

As an FYI, I've renamed timing to execution. This is so that we can leave room for adding kernel information, etc later. During the weekly meeting, we decided to make this an experimental feature - update. So, it might be good to punt on this PR until we see how the new feature plays out in lab

[MSeal]

@Madhu94 @saulshanabrook Has this change felt solidified in Lab yet? There were some conversations about how to consistently record execution timing in a couple other projects and I wound up getting to this PR. I'd be happy to help push the format improvement change through here to make it more consistent (e.g. papermill has it's own format for execution time saved in the papermill metadata field).

[Madhu94]

@MSeal Re: lab, it was a very recent change and I doubt anyone's started building on top of this yet or given any feedback. I'd be happy to have your inputs to improve the current state of things.

@saulshanabrook, do you think we should go ahead here?

[saulshanabrook]

@MSeal Yeah I think this is a good place to have that discussion. If this format seams alright to you we should figure out how to get this PR accepted.

[captainsafia]

@saulshanabrook @Madhu94 Do you have any updates?

If there is no feedback on people building on top of the notebook, have you received feedback from notebook users?

[saulshanabrook]

@captainsafia We are using this in JupyterLab! So 👍 on merging this in from our side.

[MSeal]

Going to merge this then. Do we want to formulate a patch release with this change? Is there anything else we should look at at the same time?