Cell status incorrectly marked busy by widget in different cell

[miduncan]

Application or Package Used @nteract/core @nteract/outputs

Describe the bug When interacting with a JupyterWiget, changing the value will mark the last-ran cell with the status of "Busy".

To Reproduce

1. Start with a fresh notebook
2. Create an IntSlider in the first cell

   import ipywidgets
   x = ipywidgets.IntSlider()
   display(x)

3. Create a new code cell underneath where you create a separate widget

   ipywidgets.Button()

4. Adjust the slider in the output of cell 1
5. Note that the status of cell 2 is marked as "Busy" as the widget in cell 1 communicates with the kernel, while the status of cell 1 is unchanged

Expected behavior Cell status should not be marked "Busy" when interacting with the unrelated output of another cell

Screenshots

Proposed solution We should not mark the status for a cell as busy for comm messages (which is what the jupyter widgets are using to talk to the kernel). Colab seems to follow this same behavior of widgets not changing the cell spinner, JupyterLab and Classic don't seem to show cell status.

Edit: Updated proposed solution

[miduncan]

@captainsafia @rgbkrk What do you guys think of the proposed solution? Happy to do the work, just want to make sure there is agreement on the proposal before coding it.

[miduncan]

Did some more digging on this. The issue lies in the widget manager, where we are updating the cell status.

The reason why we can't do this is because the widget manager is a static class. We give it fresh action functions every time a new widget is displayed, but for cell-specific actions such as updateCellStatus that is a problem because that means every widget will be calling this action.

So then for fixing the question becomes do we need to support this requirement of a widget setting the cell status as busy? If not, I think we can just get rid of this. If so, then we need to start tracking which cells are associated with which widgets, which I worry will introduce its own set of bugs and edge cases.

[miduncan]

Additional negative behavior of this bug: When doing a run-all where there a 2 cells with widgets one-after-the-other, it can sometimes leave the first stuck in a "busy" state. This is because we are marking cell A as "busy", then cell B creates a widget and updates that updateCellStatus, so by the time the kernel would be marking cell A as "idle" it's now marking cell B instead.

[captainsafia]

Did some more digging on this. The issue lies in the widget manager, where we are updating the cell status.

Ah, interesting. This is actually counter to what I had originally thought would be the cause which is the global cell_status handlers. But I realize those are only on the execute_requests and the comm messages are handled independently.

So then for fixing the question becomes do we need to support this requirement of a widget setting the cell status as busy?

Yeah, I think that is the correct behavior. The original implementation for the widget output only supports output and clear_output messages for the callbacks on the iopub channel.

https://github.com/jupyter-widgets/ipywidgets/blob/b62592c430200ec7c8bdb71a5d7fd8726c0e1d7c/widgetsnbextension/src/widget_output.js#L63-L83

Thanks for digging into this!

[miduncan]

We're getting some complaint on the cell being shown as busy for widget updates, as people are mistaking it for the cell executing and complaining that it is taking a long time/going weirdly back and forth (as this is what they expect with the cell status) when it is actually their widget updating a lot. Compared to JupyterLab, which doesn't show this, it can make our widgets look much slower since users mistake it as loading (and thus blocking) when it isn't.

Based on this feedback, I think it makes more sense to remove it completely. However, there could be use-cases for why we want this that I am missing. What are your thoughts?

[captainsafia]

Based on this feedback, I think it makes more sense to remove it completely. However, there could be use-cases for why we want this that I am missing. What are your thoughts?

I realized my last comment wasn't very clear. I'm OK with removing it considering the other implementation does not support it.