Unexpected behavior for "raises-exception" tag

[mgeier]

Since #2549, exceptions in cells tagged with raises-exception are ignored on "Run All".

That's good, but it didn't go far enough.

If a cell tagged with raises-exception does not actually raise an exception, this should itself be an error and stop the rest of the notebook from executing.

Related issues:

• "allow errors" vs. "raises exception" nbconvert#730
• Unexpected behavior on cells tagged with "raises-exception" computationalmodelling/nbval#108
• Allow raises-exception to continue jupyterlab/jupyterlab#2412

[vidartf]

TBH, I do not consider it the notebook's responsibility to run assertions on the output. That's what tools like nbval should do. If you want to have nbval integrate better with the front-end (e.g. a single-click button to do a test run with nbdime-powered failure reporting), I'd love to help out with that. It is in the "whishlist" part of my TODO list currently, but you seem to be pushing for the feature, so I'd be more than happy to let you lead the effort.

[mgeier]

@vidartf

If you want to have nbval integrate better with the front-end

Thanks, but no, I personally don't need this. But others might.

you seem to be pushing for the feature

Sorry if I wasn't clear, but I'm not trying to push anywhere near that (although I don't have anything against it, either).

I'm only pointing out inconsistent behavior in the notebook (and other projects).

And I'm making a concrete suggestion how to make the behavior consistent.

I do not consider it the notebook's responsibility to run assertions on the output

Probably not, but when notebook stopped ignoring the raises-exception tag in #2549, a mistake was made which I'm hoping to get fixed now.

As long as the raises-exception tag (and for that matter, any other tag) is consistently ignored, that's fine.

But if the raises-exception tag is not ignored, it should do what the name suggests, which it currently doesn't (or does only partially). If it allows exceptions, it must also forbid non-exceptions, otherwise the behavior is inconsistent with the tag name.

I strongly suggest that the behavior should be changed, but alternatively, also its name could be changed. In jupyter/nbconvert#730 I suggested allow-exception as an alternative name if the behavior really has to stay the same. But IMHO raises-exception (with the right behavior) would be much better!

Please also note another problem that I described in jupyter/nbconvert#730 (comment).

I hope I can turn your 😕 into a 😄, if you have further questions, don't hesitate to ask!

[vidartf]

[...] in #2549, a mistake was made which I'm hoping to get fixed now.

While I can agree in hindsight that the name is somewhat inaccurate, changing the name of the current tag seems like an overly disruptive change for what is more of a documentation problem. If it is agreed to rename it, then it should probably be for a major release.

To me, a better compromise would be to add another tag that does what you are currently expecting. By adding a stronger wording, like e.g. assert-error/assert-raises/assert-exception, the distinction should be made clearer to anyone that is comparing the two tag names.

Still, the best short-term solution would probably be to write documentation for tags, as well as document the existing tag (raises-exception) to make its behavior clearer.

[mgeier]

@vidartf

To me, raises-exception and assert-exception (as well as the above-mentioned alternatives) have no mutual difference in meaning. If more than one of them were available, I would have to assume that they are merely aliases of one another.

And while I agree that some documentation would be helpful, I don't think the actual problem can be solved by documentation alone.

I honestly don't understand why there is so much skepticism against treating a non-exception in raises-exception as a hard error. I can't even come up with a single situation where I would not want this behavior.

Can you give an example where creating an error on non-exception would be harmful?

Sure, changing the behavior might raise a few errors where there were none before (or not, who knows?), but those errors will actually make previously hidden bugs clearly visible, which is a good thing!

[vidartf]

Can you give an example where creating an error on non-exception would be harmful?

The current behavior reads as "if the cell raises an error, ignore it". An example can be an ImportError if an optional dependency is missing. This will unreliably raise or not.

[vidartf]

I honestly don't understand why there is so much skepticism against treating a non-exception in raises-exception as a hard error.

Also relevant from first issue comment:

If a cell tagged with raises-exception does not actually raise an exception, this should itself be an error and stop the rest of the notebook from executing.

For me the issue is insisting on treating it as a hard error, stopping further execution. That would need changes on several layers (even in the kernels?).

I would be much more open if instead the issue did not require an execution stop, but instead simply was to give some sort of indication to the user that an error was not thrown if the raises-exception tag is set. Such a solution:

• would still retain the code from allow raises-exception to continue #2549 untouched
• would put this feature purely in the front-end
• could be made in a backwards-compatible way, e.g. by not using a modal dialog, but rather some CSS styling or a pop-up error or something similar.

[mgeier]

The current behavior reads as "if the cell raises an error, ignore it".

Exactly, that's the behavior, but that's not what the tag name says. The tag name raises-exception says: "This cell raises an exception".

An example can be an ImportError if an optional dependency is missing. This will unreliably raise or not.

Thanks for the example!

But how would you reasonably use such a notebook? It's broken. There should be a visible error.

If the optional dependency is missing, the ImportError will be ignored alright, but what is supposed to happen when you use the (not) imported thing?
You will get a NameError because it wasn't imported ... and then?

To make this work, you could use one of the standard idioms that are used in such a case:

try:
    import something
except ImportError:
    something = some_alternative_thing()

Or you would define a flag like this:

try:
    import something
    something_is_available = True
except ImportError:
    something_is_available = False

And you wouldn't need the raises-exception tag in either case.

For me the issue is insisting on treating it as a hard error, stopping further execution.

OK, then I'll not insist on that.

I think it makes most sense from a user perspective to stop further execution.
But if there are technical reasons that make that very tedious to implement, I'm fine with an alternative way to report an error to the user.

The more visible the error, the better. But the actual details are up to the ones who implement the change.