Include link to the cell with an error, in failure heading

[huonw]

Papermill currently inserts an error message An Exception was encountered at 'In [NNN]' at the top of a notebook that fails to execute, where NNN is the cell execution count of the failing cell. This patch adjusts the In [NNN] text to be a link to the relevant cell, to make it easy to jump to. This works by inserting another markdown cell immediately before the failing cell, where the markdown cell contains an HTML anchor (specifically, id="papermill-error-cell"). The heading then links to it with a #papermill-error-cell fragment HREF.

This helps reduce the scrolling and searching required for long notebooks where the error cell might be one of dozens, or hidden among long output. For an example of the latter, see https://nbviewer.jupyter.org/gist/huonw/dd13fa49b5876bfea26b69832702d573.

[codecov]

Codecov Report

Merging #483 into master will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   91.42%   91.55%   +0.13%     
==========================================
  Files          14       14              
  Lines        1224     1243      +19     
==========================================
+ Hits         1119     1138      +19     
  Misses        105      105              

[MSeal]

So a downside to this approach is that it leaves this cell around after you fix issues. If we move forward with this, I think we need to tag the cell and remove cells with that tag at the start of execution. However even this will leave the cell around if someone runs with papermill and fixes the issue in a Notebook UI. They'd have to remember to manually remove the error cell.

Thoughts on this problem from other maintainers? @captainsafia @willingc ?

[huonw]

Yeah, that concern was why I chose to have the second cell have text in it, rather than just <span id="papermill-error-cell"></span>, so that it would be more obvious and more likely that people remember to remove it. I like the approach of tags and automatically remove them, when re-executed via Papermill. I'll whip up an implementation now 😄

I also looked for a Jupyter-native way of linking to specific cells, but couldn't find one. Maybe I missed it?

[MSeal]

There is not a Jupyter-native way to link to cells -- it's been a hot topic for debate which likely needs to have a cell id exposed to notebook in the next major format version bump. See jupyter/nbformat#148

Another option is to inject an html output at the end of the cell before's outputs block with the link info. This would make rerunning the notebook in any interface clear the result. Though this would appear in the prior cell's output section which might be visibly odd.

[huonw]

I'll whip up an implementation now

Done.

There is not a Jupyter-native way to link to cells -- it's been a hot topic for debate which likely needs to have a cell id exposed to notebook in the next major format version bump. See jupyter/nbformat#148

Ah, I see.

Another option is to inject an html output at the end of the cell before's outputs block with the link info. This would make rerunning the notebook in any interface clear the result. Though this would appear in the prior cell's output section which might be visibly odd.

Ah, so manually edit the output? I think this won't work well if the cell(s) before the error are markdown cells and so don't have separate output?

[MSeal]

Ah, so manually edit the output? I think this won't work well if the cell(s) before the error are markdown cells and so don't have separate output?

Yep that's true :/

[captainsafia]

This is a tricky one. The ideal implementation would store information in the metadata of the notebook and leave each UI to determine how to display this error to the user. While, IMO, that is the best from a design perspective, it's difficult to accomplish.

For compatibility across different UIS and implementation cost, this seems sensible.

Left a quick nit to update the error message so that users can track where it came from (papermill).

[MSeal]

I know this has been sitting for awhile -- I'll set some time to hash our if we want papermill to inject the link cell or not with the wider nteract group and get a decision after our next sync.

@willingc @rgbkrk fyi ☝️

[huonw]

Thanks for the update; that would be great if you happen to get to it 👍

For a little more context, I'm excited by this tweak or something equivalent, because it'll put the icing on the cake for how we are checking notebooks on CI and making them conveniently viewable: https://medium.com/stellargraph/better-notebooks-through-ci-automatically-testing-documentation-for-graph-machine-learning-5789e277e597 ("Highlighting errors with Buildkite artifacts and annotations" section in particular).

[rgbkrk]

I like the injected cell at first glance, even with the tradeoff of having to delete that cell later.

Also... kind of tangentially, I think it's ok for us to make cell IDs and stick them in cell metadata when run via papermill. Maybe the way to unblock the inertia on that effort is to start providing something UIs can work with even if it's not fully planned.

[MSeal]

Alright from talking I think we're all in agreement with @rgbkrk 's comment above. Going to merge and snap a patch release for this.

[MSeal]

Also for cell IDs... i'm going to make a JEP soon based on https://discourse.jupyter.org/t/annotating-jupyter-notebooks/2079/11

[huonw]

Thanks for merging 👍 Hopefully it doesn't take long for there to be a better way to do this!