Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ryven not working with ryvencore #126

Closed
amaury-anciaux opened this issue Nov 28, 2022 · 6 comments
Closed

Ryven not working with ryvencore #126

amaury-anciaux opened this issue Nov 28, 2022 · 6 comments

Comments

@amaury-anciaux
Copy link
Contributor

amaury-anciaux commented Nov 28, 2022

Hello,

It looks like Ryven doesn't work anymore with the new interfaces of ryvencore.

Traceback (most recent call last):
  File "C:\git\Ryven\ryven\main\Ryven.py", line 105, in <module>
    run()
  File "C:\git\Ryven\ryven\main\Ryven.py", line 26, in run
    init_node_env()
  File "C:\git\Ryven\ryven\NENV.py", line 26, in init_node_env
    from ryvencore_qt import \
  File "C:\git\ryvencore-qt\ryvencore_qt\__init__.py", line 12, in <module>
    from .src.core_wrapper import *
  File "C:\git\ryvencore-qt\ryvencore_qt\src\core_wrapper\__init__.py", line 2, in <module>
    from ryvencore import InfoMsgs, NodeInputBP, NodeOutputBP, ExecConnection, dtypes, \
ImportError: cannot import name 'NodeInputBP' from 'ryvencore' (C:\git\ryvencore\ryvencore\__init__.py)

Do you have any plans to update it? I wanted to do some tests and possibly develop a node library. The improvements to ryvencore seem to be worth it, but without a working Ryven it's a bit tough to get started!

@leon-thomm
Copy link
Owner

If you install Ryven from PyPI, it should install ryvencore v0.3, which is compatible. In case you are trying to run Ryven v3.1 on ryvencore v0.4-alpha, yes that is not compatible. I spent most of my efforts over the past few months on ryvencore, partly because ryvencore itself is becoming a dependency in other projects. I did not have capacity yet to work on Ryven (and especially ryvencore-qt) to adapt them, there is a bit of work to be done, and I don't have plans to get my hands on this in the next two months. Contributions welcome!

@amaury-anciaux
Copy link
Contributor Author

Thanks for your reply! Let me look into the code and see if there's something I can contribute, but not sure I have enough understanding of the project overall to adapt Ryven.

@amaury-anciaux
Copy link
Contributor Author

I went through some of the changes that are required to match the new ryvencore code. Broadly I saw:

  1. Removal of Connection class
  2. Removal of Script class reflected in json file format
  3. VarsManager, LogsManager and DTypes becoming addons, DTypes removed from Nodes
  4. Removal of node_view_placed event from Flow and view_place_event from Node
  5. Other minor API changes such as removal of init_data field in Flow

I will continue to explore, but wanted to ask you:

  • On (1) is it the goal to keep such a barebones interfaces as Tuple[NodeOutput, NodeInput]? I would agree with you that the original Connection object doesn't add much on top, but that will delegate some of the work to calling functions e.g. in the form of working directly with graph_adj.
  • On (4) can you explain the logic of removing view_place_event from Node? I could bring the virtual method in the Node wrapper, but want to make sure that it's the right approach.

Thanks!

@leon-thomm
Copy link
Owner

leon-thomm commented Dec 9, 2022

thanks a lot for putting this together

  • On (1) is it the goal to keep such a barebones interfaces as Tuple[NodeOutput, NodeInput]? I would agree with you that the original Connection object doesn't add much on top, but that will delegate some of the work to calling functions e.g. in the form of working directly with graph_adj.

Yeah, not sure how well this will work with the GUI, it might be useful to add the Connection abstraction again on a higher level (in ryvencore-qt) but I really didn't look into the details of that, open for suggestions. The important change is the simplification of the API in ryvencore. I'd prefer little to no abstraction in ryvencore-qt on top of ryvencore if it's not necessary (I think the graph's adjacency list should be transparently accessible).

  • On (4) can you explain the logic of removing view_place_event from Node? I could bring the virtual method in the Node wrapper, but want to make sure that it's the right approach.

If I recall correctly I just wanted to move these to ryvencore-qt because they are GUI dependent. But I did not reimplement them in ryvencore-qt yet.

@amaury-anciaux
Copy link
Contributor Author

Ok that's what I thought! I went through the changes as an exercise to better understand the project, but I can put together a PR to fix Ryven and ryvencore-qt so that it works with what you have in ryvencore on the dev branch.
I need to work through the rest of the code base though, so am not exactly sure when I'll be able to finish it.

@leon-thomm
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants