klauer / qtpynodeeditor

Python Qt NodeEditor (qtpy, PyQt5, PySide)
https://klauer.github.io/qtpynodeeditor/
Other
186 stars 53 forks source link

FIX: accidentally creating NodeDataModel twice #44

Closed klauer closed 4 years ago

klauer commented 4 years ago

Description

The initial registry .create() apparently was supposed to return (cls, kwargs) but mistakenly was returning cls(**kwargs), causing duplicate NodeDataModel instances.

This PR adds get_model_by_name which includes the intended original functionality, fixing the duplicate bug.

Motivation and Context

Closes #42

How Has This Been Tested?

Test suite continues to work. (Note that continuous integration is broken at the moment) Tested locally and saw that models are no longer created twice when created interactively (i.e., through the FlowView context menu click_handler).

cc @Picard2200 - mind giving this PR a try?

Picard2200 commented 4 years ago

Description

The initial registry .create() apparently was supposed to return (cls, kwargs) but mistakenly was returning cls(**kwargs), causing duplicate NodeDataModel instances.

This PR adds get_model_by_name which includes the intended original functionality, fixing the duplicate bug.

Motivation and Context

Closes #42

How Has This Been Tested?

Test suite continues to work. (Note that continuous integration is broken at the moment) Tested locally and saw that models are no longer created twice when created interactively (i.e., through the FlowView context menu click_handler).

cc @Picard2200 - mind giving this PR a try?

Hi Klauer, I will be pleased to test the fix.

Let me know how to get the fix (I am not a pro of git if you understand what I mean ;-) )

klauer commented 4 years ago

@Picard2200: No problem - if you're not able to try it, I'll make an attempt to get this and the other open PR out in a new release in a day or two.

If you get a chance, give this a shot if on conda:

$ conda create --clone qtpynodeeditor_test_env (your_current_env)
$ conda activate qtpynodeeditor_test_env
$ git clone --single-branch --branch fix_model_dupe https://github.com/klauer/qtpynodeeditor
$ cd qtpynodeeditor
$ pip install -e .

Then run your old script.

Otherwise if not on conda, you could temporarily uninstall qtpynodeeditor and then install this branch:

$ pip uninstall qtpynodeeditor
$ git clone --single-branch --branch fix_model_dupe https://github.com/klauer/qtpynodeeditor
$ cd qtpynodeeditor
$ pip install -e .
Picard2200 commented 4 years ago

Hi Klauer, I have run the command conda create -n qtpynodeeditor_test_env --clone base switched to this new env but was not able to access the specifed branch, fix_model_dupe. I assume that you have used it for test and deleted it.

I take advantage of this message for the work you have done and for your short delay to answer this issue.

Thanks to your example, calculator.py, I was able to learn plenty of things like how to deal with ports of different types.

Thank you again

klauer commented 4 years ago

@Picard2200 - it's now in a release, so you should be able to install it normally. Please do let me know what else you run into - and hope you have fun with the library.

Picard2200 commented 4 years ago

Hi Klauer, I still have the issue on the version I got today. I got the last version and ran "python setup.py install" on my conda environment. To check whether it is fixed, I add a print in the MathOperationModel init method and I see twice the message when I add a new operation using calculator.py (left click on the scene)

Pycharm, marvelous editor, reported an error in node.py line 282 with missing closing >') Updated comment: actually, when I edit the file using Notepad++, I do not have this issue so it seems to be a PyCharm issue: sorry

Picard2200 commented 4 years ago

I have a lot of fun with the library and I am making advertisement for it :-)

klauer commented 4 years ago

@Picard2200 - hmm, strange. I'll reopen your issue. Let's also move the discussion back there...