Updates to ryvencore API

[amaury-anciaux]

Following discussion in #126 this allows Ryven and ryvencore-qt to use the current dev branch of ryvencore. I implemented the ryvencore API changes (removal of Script, connections, view_place_event, load_data), updated the widgets and wrappers and fixed references to the addons so that Ryven can start and one is able to drop new nodes (tested only with Val and Results).
This is a first step, and will require some more work as connections don't fully work yet, to see if you're okay with the direction.
There's a few open questions:

• The input widgets now don't have an object to store a value in (only output ports do so in the new ryvencore), so this is where I stopped. I was thinking this may be the moment to completely remove the input widgets, and store all the interactivity in the main Node widget.
• I think I didn't follow your logic for imports, which would in any case benefit from some clarification/simplification.
• Pushing the simplification thinking, I also wonder what use case there is for the Console in Ryven? This is what drives some complexity in the imports, and I'm not sure who would use that very bare-bones execution mode.

[leon-thomm]

Thanks a lot for starting work on this.

• The input widgets now don't have an object to store a value in (only output ports do so in the new ryvencore), so this is where I stopped. I was thinking this may be the moment to completely remove the input widgets, and store all the interactivity in the main Node widget.

Good point. Also, an enabled input widget is equivalent to having a separate node with a main widget producing the data as output that is connected to the input. I've seen numerous use examples where input widgets as they are available now were used extensively, so I'm not sure about removing them. But it is correct that the fundamental assumption in the ryvencore dev version is currently that inputs are completely stateless, maybe this has to be relaxed, I'm not sure, open to further suggestions...

• I think I didn't follow your logic for imports, which would in any case benefit from some clarification/simplification.

imports of what?

• Pushing the simplification thinking, I also wonder what use case there is for the Console in Ryven? This is what drives some complexity in the imports, and I'm not sure who would use that very bare-bones execution mode.

Ryven Console is simply interactive headless mode. It's supposed to provide an easy way to deploy flows headless (directly on ryvencore, hence no Ryven dependencies) and to take care of all mangling that might be required. For example, the option to deploy Ryven flows in an environment that has no graphics support is quite important in many scenarios. Why does it drive complexity?

[amaury-anciaux]

On input widgets:

I'm actually thinking that saving the widget's value in a ryvencore object is not needed at all, since the input() method of the ryvencore-qt Node wrapper actually handles the widget value if it exists. I would need to check in more detail what else in the current version of ryvencore-qt interfaces with the NodeInput's value directly and if they could also be removed. What do you think?

This means that conceptually the InputWidget would not match a ryvencore element, and can be considered as you were saying as a separate node, or as I'm arguing just as a convenient way to design a UI for inputs without having to develop a MainWidget and all the plumbing that goes with it.

The other two points are more minor and may result in additional updates later:

On the imports, it related to the logic behind the imports from different packages and the aliases used. It took me some time to understand them. It may just be a matter of giving them more consistent aliases throughout.

On the console, I had the feeling that the value that Ryven provides to run ryvencore flows headless would be more in providing well-structured helper functions to import packages and load projects, not necessarily an interactive command-line app. To your question, complexity here is just maintaining the RyvenConsole code, which could be deleted if nobody uses it.
Separately the helper functions may be refactored for clarity in the future.

[leon-thomm]

I'm actually thinking that saving the widget's value in a ryvencore object is not needed at all, since the input() method of the ryvencore-qt Node wrapper actually handles the widget value if it exists. I would need to check in more detail what else in the current version of ryvencore-qt interfaces with the NodeInput's value directly and if they could also be removed. What do you think?

Everything that changes the semantics of a node must be represented on ryvencore level. Otherwise, the node breaks when deploying on ryvencore only, which defeats the purpose of ryvencore. ryvencore-qt is only supposed to do GUI, no flow semantics.

Adding a ryvencore-level representation could go hand in hand with a "default value" feature that was requested somewhere, the value provided by an input widget is essentially the same (if a connection is present, the widget value is ignored)...

On the imports, it related to the logic behind the imports from different packages and the aliases used. It took me some time to understand them. It may just be a matter of giving them more consistent aliases throughout.

I'm really not the best at naming stuff so feel free to suggest changes here 👍

On the console, I had the feeling that the value that Ryven provides to run ryvencore flows headless would be more in providing well-structured helper functions to import packages and load projects, not necessarily an interactive command-line app.

I'm not sure if some programmatic API interface for that alone would be very helpful. The console (despite in desperate need of extensions and improvements) has proven to be useful in many cases for me and others already, I don't think it doesn't have a use case.

[amaury-anciaux]

This makes a lot of sense, and I like the default value approach. I'll be able to move forward with the updates in -qt when you merge leon-thomm/ryvencore#41.

[leon-thomm]

👍 merged it

[amaury-anciaux]

I was finally able to re-start on this, I've identified an issue in ryvencore at the same time (leon-thomm/ryvencore#43). I realize there might be more work than I thought (e.g. need to implement using the Data object), so I'm chipping away bit by bit and have added a few commits here and in -qt.
One thing you may be able to help: PortItem relies on connecting to an event from the defunct ryvencore connection object. Given that doesn't exist anymore since all the flow logic is managed in FlowExecutor, what do you think is the best approach? I can think of re-implementing an Event in a wrapper for NodePort, any other idea?

[leon-thomm]

PortItem relies on connecting to an event from the defunct ryvencore connection object.

As far as I can see, this was only used to display new input values in the widget. There's a few possible solutions to this.

I have been doing some experimental work, based on your changes, on ryvencore_upgrade_testing in -qt. In particular, I just removed the wrapper classes that subclassed and re-exposed ryvencore components (Session and Node), and instead implemented SessionGUI and NodeGUI which act as interface between ryvencore and the qt GUI and implement the same functionality. This separates responsibilities better and makes the API more explicit.

Going with this approach, a possible solution would be what I indicated in the above commit.

[amaury-anciaux]

Yes, I like the approach, at times I was getting lost trying to find who does what between ryvencore and ryvencore-qt wrappers. Looks like you're actively working on this one, so let me know when things are stable, happy to continue to help. In the meantime I quicky tried to rerun Ryven, but it looks like there will be some refactoring to do there.

In -qt were two commits I did after you branched out that probably need to be rebased:

• leon-thomm/ryvencore-qt@bd757b9
• leon-thomm/ryvencore-qt@4ccd881

[leon-thomm]

In -qt were two commits I did after you branched out that probably need to be rebased: [...]

I looked at those changes, some don't quite apply anymore after my changes, the rest I just integrated (slightly modified) in the respective commits

• leon-thomm/ryvencore-qt@2166db4 and
• leon-thomm/ryvencore-qt@a3b695f

(hence no need to rebase your two commits, I think). The approach with the GUI classes seems to be working, so if everything looks good to you, I can merge ryvencore_upgrade_testing into dev. Next points to get Ryven up to date would be

• integration of dtypes in -qt, presumably as ryvencore AddOn
• refactoring Ryven to make compatible with ryvencore-qt again
• PySide6 compatibility (finally??)

and probably fixing loads of small issues in ryvencore-qt along the way. Once the first two points are mostly done we can aim for an alpha release of a new Ryven version very soon.

[amaury-anciaux]

Looks good to me on the -qt side! I'll try to keep working on Ryven, but will have more limited time in the next months. So maybe you merge this PR in a new branch to avoid it lasting for tool long?

[leon-thomm]

Changes started in this PR are still WIP, I'll continue work on branch ryvencore_upgrades.