Indicate and show exceptions in the flow diagram

[sphh]

I was looking for a framework like Ryven for a long time after they stopped developing VisTrail. I like it very much! Thanks for making this available!!

At the moment, the following is possible in Ryven 0.3.1.0: This state is reached after changing the second value from 1 to 0 and is obviously wrong.

If I enable Info Messages I can see the ZeroDivisionError, but this is not reflected in the flow path itself. What is worse, if I have more than one / node, I would not even know, which node is affected.

IMHO it would be really good, if exceptions raised during update() are displayed in the GUI, such as: (Alternatively display a warning icon ⚠ somewhere in the node.)

As you can see, all affected nodes should also be indicated, possibly not only by a warning path, but also by a warning border (not shown in the picture above and possibly with a slightly different color).

Now if you move the mouse over the node (or the warning icon), the error message (but not the whole traceback) is displayed together with the __doc__ string.

This should be handled natively by the Node class itself without any programming needed by the custom node's developer.

As a more sophisticated approach, the node itself could set the input ‘bullet’, which caused the exception, to a warning colour before raising the exception. Obviously this would be the task of the node's developer.

What do you think?

[leon-thomm]

Great suggestion!

the above commit adds a very basic implementation of automatic error during node update to ryvencore-qt.

As you can see, all affected nodes should also be indicated, possibly not only by a warning path, but also by a warning border (not shown in the picture above and possibly with a slightly different color).

I think I would not try to depict affected nodes or connections, as this can vary, it's generally not really clear what nodes are affected (depending on your nodes programming your node might intentionally only fail when trying to set a specific output, while a node update also does not imply that all output connections will be updated if the nodes executes successfully).

As a more sophisticated approach, the node itself could set the input ‘bullet’, which caused the exception, to a warning colour before raising the exception. Obviously this would be the task of the node's developer.

I agree, something like that could be added to the API but I didn't go into details of that so far

[sphh]

Thanks! That looks really great.

I applied your commit https://github.com/leon-thomm/ryvencore-qt/commit/48bab818bfacd4a5364b8e27b178c7fa15a27534, but the exception is not shown. I then tried to call self.update_error(e) from inside a Node and I get the following exception:

  File "/usr/local/lib/python3.8/dist-packages/ryvencore_qt/src/core_wrapper/Node.py", line 108, in update_error
    RC_Node.update_error(self, e)
AttributeError: type object 'Node' has no attribute 'update_error'

Is there another commit, I missed and should have applied?

I think I would not try to depict affected nodes or connections, as this can vary, it's generally not really clear what nodes are affected (depending on your nodes programming your node might intentionally only fail when trying to set a specific output, while a node update also does not imply that all output connections will be updated if the nodes executes successfully).

I see. Would it be possible to pass a Failed object to an output, which is recognized by the input port? Then the node's programmer can handle that one however s/he thinks is most appropriate?

I agree, something like that could be added to the API but I didn't go into details of that so far

If you look into it, the dtypes.Float etc. could also mark the input as invalid …

[leon-thomm]

Is there another commit, I missed and should have applied?

yes, sorry I had to add the update_error() function to the core, so you also need to update ryvencore (commit: https://github.com/leon-thomm/ryvencore/commit/a7529054d94b6705ef87e355d4944b313a80b092). for this kind of development stuff I suggest cloning all repos (Ryven, ryvencore-qt, and ryvencore) so you have the dev branch available for all of those, and reinstalling e.g. ryvencore simply via

(venv) .../ryvencore>pip uninstall ryvencore
(venv) .../ryvencore>pip install .

when needed.

Would it be possible to pass a Failed object to an output, which is recognized by the input port? Then the node's programmer can handle that one however s/he thinks is most appropriate?

Yes, but that would introduce yet another condition to be checked on every arrival of any data on any input in any node, and since it all runs in python we may want to keep an eye on performance.

If you look into it, the dtypes.Float etc. could also mark the input as invalid …

true, the dtypes system is perfect for that.

[sphh]

yes, sorry I had to add the update_error() function to the core, […]

That makes absolute sense. Thanks.

Still the icon and tooltip is not shown. I checked and NodeErrorIndicator.show() is called, but the icon does not appear.

Yes, but that would introduce yet another condition to be checked on every arrival of any data on any input in any node, and since it all runs in python we may want to keep an eye on performance.

Does one simple is check per input really takes that long? My idea was at follows:

# Setup an error object - global:
RyvenError = object()
# or as a singleton, so that it can be easily imported:
class _Singleton(type):
    """A metaclass for singletons."""

    _instances = {}

    def __call__(cls, *args, **kwargs):
        """Return the existing instance if already instanciated."""
        if cls not in cls._instances:
            cls._instances[cls] = super(_Singleton, cls).__call__(
                *args, **kwargs)
        elif args:
            raise ValueError(
                f"Class '{cls.__class__.__name__}' already initiated.")
        return cls._instances[cls]

class RyvenError(metaclass=_Singleton):
    pass

In the standard Node.update_event(), something like this might work:

# Check, if an input got an error object
for value in inputs:
    #  'is' is faster than '=='
    if value is RyvenError:
        # All outputs depends on correct inputs
        for output_no in range(len(outputs)):
            self.set_output_val(output_no, RyvenError)
        return
# Proceed without error

but I guess, I'm too naïve, because I do not know the internal working of ryvencore …

The reason, I'm asking for this feature, is that errors, which are outside of the focused area otherwise would easily go undetected. Alternatively a global error icon could be displayed - but with a large flow network, you then would start searching. But still it would be much better than missing one.

[leon-thomm]

Still the icon and tooltip is not shown. I checked and NodeErrorIndicator.show() is called, but the icon does not appear.

that's strange, does Qt print any errors to the console?

# Check, if an input got an error object
for value in inputs:
    #  'is' is faster than '=='
    if value is RyvenError:
        # All outputs depends on correct inputs
        for output_no in range(len(outputs)):
            self.set_output_val(output_no, RyvenError)
        return
# Proceed without error

yeah, I'm afraid this would get quite tricky. this code assumes that the graph is a tree, i.e. once you have a feedback loop in your graph (which can indeed be intended, though it obviously requires a very thought-out design of your nodes), this would result in an infinite loop. However, I did try to implement some generic feedback loop checker before and it turned out much harder than I expected (basically impossible without assumptions about node behavior).

The reason, I'm asking for this feature, is that errors, which are outside of the focused area otherwise would easily go undetected. Alternatively a global error icon could be displayed - but with a large flow network, you then would start searching. But still it would be much better than missing one.

I agree, I think a global sign that pops up would be the most straightforward approach. Maybe we can add a function zoom_on_node(n: Node) to ryvencore-qt's FlowView which makes the viewport jump to the node causing the exception when clicking on the pop-up sign.

[sphh]

that's strange, does Qt print any errors to the console?

Nothing in the message window (with info messages enabled) and nothing in the terminal, I started ryven from. But please do not worry at the moment, maybe I made a stupid mistake when patching the files (I have not cloned and installed the repository yet).

yeah, I'm afraid this would get quite tricky. this code assumes that the graph is a tree, i.e. once you have a feedback loop in your graph […], this would result in an infinite loop.

Aargh!, I haven't thought of that! Would the loop break, if the output is not set whenever its value is already RyvenError?

I think a global sign that pops up would be the most straightforward approach. Maybe we can add a function zoom_on_node(n: Node) to ryvencore-qt's FlowView which makes the viewport jump to the node causing the exception when clicking on the pop-up sign.

Now that is A Good Idea! Maybe a big warning sign on the background of the FlowView, which cannot be missed?

[leon-thomm]

Aargh!, I haven't thought of that! Would the loop break, if the output is not set whenever its value is already RyvenError?

hm, can't say why this would not work right now, but I'm a bit sceptical... might be worth a try though

Maybe a big warning sign on the background of the FlowView, which cannot be missed?

I was thinking of a notification-like overlay widget, but painting some warning watermark in the background should also be possible.