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 udates #18

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Ryven udates #18

wants to merge 6 commits into from

Conversation

leon-thomm
Copy link
Contributor

Hey, I'm currently trying to move towards a pre-release of a new Ryven version (see Ryven dev) to integrate a largely reworked ryvencore (docs here) which I wrote throughout last year. There are many breaking API changes.

  1. Stronger separation of node functionality and GUI. I'd like node packages to be compatible with headless mode by default (no GUI dependencies, no Qt). And I think the new system makes that refreshingly easy.
  2. A conceptual change in nodes programming: any data sent over node ports must be wrapped in a Data object which defines how the data can be de-/serialized (i.e. when the object sent is not pickle serializable by default - like pythonocc types - then the package developer has to define de-/serialization manually). This finally enables Ryven to efficiently and correctly re-establish the old graph state when loading a project, even when stateful nodes are used, which was not the case before!
  3. And many little API improvements which I included because why not if I'm already breaking stuff.

I thought I might use this package to 1. find the pain points of upgrading, and 2. working out the details of the API changes. So I just did that. Some important points:

  • A Node subclass class can now reference a custom NodeGUI subclass which implements everything UI related now (color, display title, widgets, right-click actions, etc.).
  • dtypes don't doesn't exist anymore. I added another system for frequently used input widgets in Ryven. Normally you would now write something like
    class MyNodeGui(NodeGUI):
        # register MyQtWidgetClass as input widget class for the node of this gui
        input_widget_classes = {'mywidget': SomeInputWidgetClass}
        # instantiate the input widget for the first initial input
        init_input_widgets = {0: 'mywidget'}
    And SomeInputWidgetClass could be either manually defined, or one of the default provided ones by Ryven (replacing dtypes). Because there is little variation of input widgets in this package, I implemented a decorator function which automatically adds this here, so we don't have to write it for every node explicitly.
  • Packages like this one could substantially benefit from the new data sharing approach. One could now implement explicit de-/serialization for all the pythonocc types that are sent from nodes to other nodes. I defined a dummy OCCData(Data) class which currently does nothing - so it does not implement de-/serialization, but that might be fine here, after loading the user just needs to update the nodes that have no input connections and provide everything else with data. But that is just an idea, we might also provide proper de-/serialization. See comment in the code.

Basic node functionality is already working again. I will also try to write some sort of script that translates old project files so they can be loaded with the new version. I'll try to wrap this up soon.

Comment on lines +125 to +126
# TODO: Edge import causes crash
# from OCCUtils.edge import Edge
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import crashes, even when I just import that in an empty python shell:

> python
Python 3.9.9 | packaged by conda-forge | (main, Dec 20 2021, 02:41:03) 
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from OCCUtils.edge import Edge
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen zipimport>", line 259, in load_module
  File "/home/leon/apps/anaconda3/envs/pyoccenv_test/lib/python3.9/site-packages/OCCUtils-0.1.dev0-py3.9.egg/OCCUtils/edge.py", line 26, in <module>
  File "/home/leon/apps/anaconda3/envs/pyoccenv_test/lib/python3.9/site-packages/OCC/Core/GeomLib.py", line 18, in <module>
    from . import _GeomLib
ImportError: /home/leon/apps/anaconda3/envs/pyoccenv_test/lib/python3.9/site-packages/OCC/Core/_GeomLib.so: undefined symbol: _ZN7GeomLib13FuseIntervalsERK18NCollection_Array1IdES3_R20NCollection_SequenceIdEd

pythonocc-core version: 7.5.1

@Tanneguydv
Copy link
Owner

Tanneguydv commented May 3, 2023

Wow it seems to be some interesting features here, thanks!

I can't make the dev branch running on my computer though, I tried to reinstall using pip install . but I get this error :

ERROR: No matching distribution found for ryvencore==0.4.*

@leon-thomm
Copy link
Contributor Author

Yeah, you need to install ryvencore and ryvencore-qt from the corresponding dev branches as well. You can clone Ryven, ryvencore, and ryvencore-qt, and in each checkout the dev branch and pip install ..

@Tanneguydv
Copy link
Owner

Tanneguydv commented May 4, 2023

Ok thank you.
but I still can't make Ryven works : I have several errors, I tried to comment the lines causing troubles but there are a lot.
Example :

check_connection_validity_request = Signal((NodeOutput, NodeInput), bool)
TypeError: pyqtSignal() argument expected to be sequence of types

when commenting the line (line 41 in ryvencore_qt\src\flows\FlowView.py) The startup dialog is displayed, when I click OK without changing anything I get :

File "...\ryven\gui\uic\ui_main_window.py", line 25, in setupUi
    MainWindow.setSizePolicy(sizePolicy)
TypeError: arguments did not match any overloaded call:
  setSizePolicy(self, a0: QSizePolicy): argument 1 has unexpected type 'PySide2.QtWidgets.QSizePolicy'
  setSizePolicy(self, hor: QSizePolicy.Policy, ver: QSizePolicy.Policy): argument 1 has unexpected type 'PySide2.QtWidgets.QSizePolicy'

When I comment the line and rerun I get:

 File "C:\ProgramData\Miniconda3\envs\kuka_iiwa_python-env\lib\site-packages\ryven\gui\uic\ui_main_window.py", line 26, in setupUi
    self.actionImport_Nodes = QAction(MainWindow)
TypeError: 'PySide2.QtWidgets.QAction' called with wrong argument types:
  PySide2.QtWidgets.QAction(MainWindow)
Supported signatures:
  PySide2.QtWidgets.QAction(PySide2.QtGui.QIcon, str, typing.Optional[PySide2.QtCore.QObject] = None)
  PySide2.QtWidgets.QAction(typing.Optional[PySide2.QtCore.QObject] = None)
  PySide2.QtWidgets.QAction(str, typing.Optional[PySide2.QtCore.QObject] = None)

It's too specific for my knowledge of Qt framework unfortunately.

One comment on your startup dialog : we can save config file but we can't load one, what is the use of it? It will be loaded when loading a .json session file?

@leon-thomm
Copy link
Contributor Author

check_connection_validity_request = Signal((NodeOutput, NodeInput), bool)
TypeError: pyqtSignal() argument expected to be sequence of types

This seems to be a PyQt error, but PyQt shouldn't be used at all, it's not supported. Do you have PyQt installed? If so can you uninstall it and try again?

  setSizePolicy(self, a0: QSizePolicy): argument 1 has unexpected type 'PySide2.QtWidgets.QSizePolicy'

So this is clearly a contradiction, or the two types come from different libraries. Maybe try creating a fresh virtual environment and installing the dependencies again (the Ryven libs from sources and PySide2, pyqt, and 'python-occ`)?

One comment on your startup dialog : we can save config file but we can't load one, what is the use of it? It will be loaded when loading a .json session file?

You can certainly load config files, the default config file ~/.ryven/config.cfg is loaded automatically, and you can specify further config files on the command line, e.g. ryven @~/.ryven/custom.cfg.

@Tanneguydv
Copy link
Owner

Indeed I had modules conflicts, it works with a new env, thank you.
I had to change some imports to make it work, example :

ModuleNotFoundError: No module named 'ryvencore.addons.default'
-->
from ryvencore.addons.Variables import VarsAddon

I can now load pythonocc nodes but can only display nodes without input :
image

@leon-thomm
Copy link
Contributor Author

Hmm, interesting. Sorry for the struggle, there's a few things to consider, I should probably figure out a better way to make everything work together.

  • Are you sure you checked out and pulled dev in the ryven repositories and installed from there? I just pushed some changes that might make it work for you so please pull the dev branches again.
  • Make sure to uninstall all ryven libraries (ryvencore, ryvencore-qt, ryven) before installing from source.
    • you can verify using conda list which lists all installed libraries
  • You need to install ryvencore first, then ryvencore-qt, and then Ryven.

Please let me know if it still doesn't work.

@Tanneguydv
Copy link
Owner

Yes, I've uninstalled and repull the dev branch, it still doesn't work.
Also to be able to run Ryven I had to change de setup.cfg file from Ryven from

install_requires =
    ryvencore-qt ==0.4.*

to

install_requires =
    ryvencore-qt

as the env is a new one and I've installed the packages in the correct order it doesn't change anything in our problem.
Just to say that I think that ryvencore-qt/dev has not the 0.4 version specified (that's a change that I had also made in my earlier tests).

@leon-thomm
Copy link
Contributor Author

leon-thomm commented May 5, 2023

I think that ryvencore-qt/dev has not the 0.4 version specified (that's a change that I had also made in my earlier tests).

It does. If it doesn't for you then your pull was not successful.

@Tanneguydv
Copy link
Owner

Tanneguydv commented May 5, 2023

My bad, VsCode displayed "dev" even after repulling but without success, deleting the dir and cloning/checkout dev had make it appeared. I still can't add nodes with inputs unfortunately

@leon-thomm
Copy link
Contributor Author

What's the error? Starting Ryven as ryven -v will give verbose output to the console, including all error messages.

@Tanneguydv
Copy link
Owner

I don't get any messages drag/droping these inputs nodes (ex: Point).

For Point0 I get:
--> INFO: setting output 0 in Point0

When I right click and choose one by double clicking on it I get:

Traceback (most recent call last):
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\FlowView.py", line 794, in create_node__cmd
    self._push_undo(
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\FlowView.py", line 260, in _push_undo
    cmd.activate()
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\FlowCommands.py", line 34, in activate
    self.redo()
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\FlowCommands.py", line 40, in redo   
    self.redo_()
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\FlowCommands.py", line 98, in redo_  
    self.node = self.flow.create_node(self.node_class)
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore\Flow.py", line 233, in create_node
    self.add_node(node)
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore\Flow.py", line 262, in add_node
    self.node_added.emit(node)
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore\Base.py", line 77, in emit
    cb(*args)
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\FlowView.py", line 817, in add_node  
    item.initialize()
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\nodes\NodeItem.py", line 112, in initialize
    self.add_new_input(inp)
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\nodes\NodeItem.py", line 189, in add_new_input
    item = InputPortItem(self.node_gui, self, inp, input_widget=widget)
  File "C:\Users\tanneguydv\.conda\envs\ryven_dev-env\lib\site-packages\ryvencore_qt\src\flows\nodes\PortItem.py", line 113, in __init__
    if self.port.load_data is not None and self.port.load_data['has widget']:
AttributeError: 'NodeInput' object has no attribute 'load_data'

@leon-thomm
Copy link
Contributor Author

leon-thomm commented May 5, 2023

I don't get any messages drag/droping these inputs nodes (ex: Point).

true, that's a bug I still need to fix, I remember.

Regarding your error: it still doesn't seem to have dev pulled correctly, I added the fix for that this morning and commited it to dev, see this commit.

@Tanneguydv
Copy link
Owner

Hey,

sorry for the pulling trouble that I had!
I have been able to re-do the torus example with success (except for the final fillet operation, don't know why) :

image

As you can see we have a result, thank you!

Some notes :

  • Nodes's color are all yellow, even though it's specified in guis
  • Some links doesn't refresh when we unplug it (example in screenshot)
  • I cannot add input to nodes (List node for example), so I had to create several List nodes with only 2 input per nodes and then add them together and flatten them
  • Perhaps an autosave function in .json format could be useful, mainly with PythonOCC where a wrong plug on input can causes crashes (as there are no try/except on nodes for the moment)

I have tried to reload the saved file in the .json format. the graph does appear well but, as you said above, the nodes are not filled with values, I need to refresh it and I can't find a way. Even node without input, such as AxZ doesn't output any values. I have tried the right-click "update shape" without success. But it is still a great improvement regarding previous loading causing crash without any display of any kind.

@leon-thomm
Copy link
Contributor Author

I have been able to re-do the torus example with success

Great!

  • Nodes's color are all yellow, even though it's specified in guis
  • I cannot add input to nodes (List node for example), so I had to create several List nodes with only 2 input per nodes and then add them together and flatten them

Thanks, fixing it now.

* Some links doesn't refresh when we unplug it (example in screenshot)

Thanks.

* Perhaps an autosave function in` .json` format could be useful, mainly with PythonOCC where a wrong plug on input can causes crashes (as there are no `try/except` on nodes for the moment)

Autosave would be neat but the implementation is a bit more difficult than it sounds. In any case there shouldn't be crashes, or the nodes package is malfunctional (which renders autosave useless anyway). The nodes cannot crash the application on update by raising an exception, exceptions should be catched by Ryven. When does this happen for you, and what is the output? (run ryven --verbose to get full output on console)

I have tried to reload the saved file in the .json format. the graph does appear well but, as you said above, the nodes are not filled with values, I need to refresh it and I can't find a way.

The way I did this now implies that all output values will be None after loading. In cases where there are few sources of truth (like a couple of val nodes and all other nodes depend on them directly or indirectly, so the whole graph will be re-executed once these val nodes update) the user can easily bring the graph back into the original state by updating those. But when there are nodes without inputs this cannot work, of course.

The default approach would be to define de-/serialization. I saw that PythonOCC doesn't support any serialization by itself, and it might be some work. Do you know anything about this?

leon-thomm added a commit to leon-thomm/ryvencore-qt that referenced this pull request May 13, 2023
@Tanneguydv
Copy link
Owner

Autosave would be neat but the implementation is a bit more difficult than it sounds. In any case there shouldn't be crashes, or the nodes package is malfunctional (which renders autosave useless anyway). The nodes cannot crash the application on update by raising an exception, exceptions should be catched by Ryven. When does this happen for you, and what is the output? (run ryven --verbose to get full output on console)

I have tried to reload the saved file in the .json format. the graph does appear well but, as you said above, the nodes are not filled with values, I need to refresh it and I can't find a way.

The way I did this now implies that all output values will be None after loading. In cases where there are few sources of truth (like a couple of val nodes and all other nodes depend on them directly or indirectly, so the whole graph will be re-executed once these val nodes update) the user can easily bring the graph back into the original state by updating those. But when there are nodes without inputs this cannot work, of course.

The default approach would be to define de-/serialization. I saw that PythonOCC doesn't support any serialization by itself, and it might be some work. Do you know anything about this?

Ok thank you very much!
I have other projects where serialization is needed for Pythonocc, I will try do do something about it as soon as I can, i keep you informed.

@leon-thomm
Copy link
Contributor Author

Quick update: I finished all major Ryven adaptions and the new versions (Ryven v3.4, ryvencore-qt v0.4, ryvencore v0.4) are available as pre-release on PyPI. In a fresh virtual environment, you can run:

pip install --pre ryven

@leon-thomm
Copy link
Contributor Author

Hey, any progress on this? We will need to define serialization and de-serialization for all the PythonOCC types passed between nodes. Is there anything I can help with that doesn't require PythonOCC knowledge? Or is there anyone who has experience with this task and could maybe help out?

@HakanSeven12
Copy link

#19 @leon-thomm

@leon-thomm
Copy link
Contributor Author

#19 @leon-thomm

not sure how your PR solves the serialization issue described here. there is no point as long as that's not solved I think

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

Successfully merging this pull request may close these issues.

None yet

3 participants