Allow user to specify custom nodes path #166
Conversation
|
@jan-janssen Is this ready to merge? I'll try to integrate the open changes and Jörg's latest updates into master today. |
Yes, I was just waiting for somebody to review it. |
| import pyiron_core.pyiron_nodes as pyiron_nodes | ||
|
|
||
| root_path = pyiron_nodes.__path__[0] | ||
| self.node_path = "pyiron_core" |
There was a problem hiding this comment.
This seems to be inconsistent with the logic in the other branches or am I missing something?
pyiron_nodes.__path__[0] would evaluate e.g. to /home/ponder/science/phd/dev/pyiron_core/pyiron_core/pyiron_nodes and in the other branches we take the last part of that part, so pyiron_nodes rather than pyiron_core. Shouldn't it be this then?
| self.node_path = "pyiron_core" | |
| self.node_path = "pyiron_nodes" |
There was a problem hiding this comment.
While I agree that it would be more consistent to use pyiron_nodes this would break backwards compatibility to all previously saved workflows. The current changes introduce new functionality - the option to specify a user defined path - without breaking the backwards compatibility. If no path is specified the code falls back to the previous behavior.
There was a problem hiding this comment.
Oh, I see, it's because pyiron_nodes is distributes in the pyiron_core package. So we need the import paths from pyiron_core, but in the node library browser we want to show only from pyiron_nodes downwards. When using a user supplied node path we assume both will be the same.
There was a problem hiding this comment.
As I said it is not the ideal solution from my perspective, but it was the previous behavior so I do not want to change it at the moment.
|
|
||
| def on_click(self, node): | ||
| path = ( | ||
| get_rel_path_for_last_occurrence(node.path.path, "pyiron_core") |
There was a problem hiding this comment.
@pmrv It is basically just used here - so before it was pyiron_core and now it is again set to pyiron_core.
pmrv
left a comment
pmrv
left a comment
There was a problem hiding this comment.
I pushed a cosmetic commit to simply the root_path if/else, but otherwise everything seems to work in my testing.
Any chance of adding the tests to the PR? |
|
I'm not sure what part to test, since I just checked visually whether the correct things are in the node library and I don't know which parts to call to check that programmatically. |
No description provided.