add colors for gui

[Tara-Lakshmipathy]

No description provided.

[github-actions]

👈 Launch a binder notebook on branch pyiron/pyiron_workflow/gui_colors

[samwaseda]

Can you add an example figure?

[samwaseda]

I find it nice that @liamhuber so far mostly followed the convention of naming the color and return the variable. Do you maybe want to do that as well?

[Tara-Lakshmipathy]

I added variables for the colors, but did not define them in the Seaborn class (which is in pyiron_snippets). I think the Seaborn colors follow some pre-defined color palette . Is that OK?

[Tara-Lakshmipathy]

And where would you like the example picture?

[samwaseda]

In the PR comment.

Actually you might also need to explain either in the code (better) or in the PR how this property is used.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.15% (target: -1.00%)	✅ 50.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (d8728f1)	3371	3088	91.60%
Head commit (0087438)	3383 (+12)	3094 (+6)	91.46% (-0.15%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#486)	12	6	50.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

[coveralls]

Pull Request Test Coverage Report for Build 11234413414

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 10 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.1%) to 91.457%

---

Files with Coverage Reduction	New Missed Lines	%
nodes/function.py	3	95.45%
nodes/composite.py	3	91.71%
nodes/transform.py	4	97.13%

Totals
Change from base Build 11097297857:	-0.1%
Covered Lines:	3094
Relevant Lines:	3383

---

💛 - Coveralls

[Tara-Lakshmipathy]

The properties are used in pyiron_xyflow here to color the nodes.

[samwaseda]

can you maybe also say real quick what was the problem without having gui_color?

[Tara-Lakshmipathy]

The colors in the gui are taken directly from the nodes in pyiron_workflow which are themselves defined in colors.py in pyiron_snippets. These colors are too dark for the gui.

[samwaseda]

Ok that's a valid concern. But I have a different concern now: Since we rely on auto-complete as well as __setattr__ and __getattr__, it might be preferable to keep the number of attributes as low as possible. Maybe the easiest solution for now is to change color by get_color and take an argument for GUI. However, I have the feeling that the real source of the problem is the fact that the node objects (nodes, macros etc.) have colors. Maybe it's really time to think about changing this.

Let's wait for @liamhuber and see what he says

[Tara-Lakshmipathy]

Ok that's a valid concern. But I have a different concern now: Since we rely on auto-complete as well as __setattr__ and __getattr__, it might be preferable to keep the number of attributes as low as possible. Maybe the easiest solution for now is to change color by get_color and take an argument for GUI. However, I have the feeling that the real source of the problem is the fact that the node objects (nodes, macros etc.) have colors. Maybe it's really time to think about changing this.

Let's wait for @liamhuber and see what he says

There may be more such attributes for the automated positioning of nodes in the gui in the future. If not in the specific types of nodes (e.g., function.py) then at least in node.py. So, maybe it's already worth discussing where such attributes should go.

[liamhuber]

I am definitely against introducing a new field for the gui color -- the core infrastructure shout have no idea the gui exists, and @samwaseda is correct about tab completion menu crowding.

I have no objection to updating the existing color attributes and that should solve the problem.

I am ok totally getting rid of the color node attribute, but this information can't be destroyed -- only moved. Somewhere we should have a table relating classes to colors, and a function for getting most-specific class color from this table given the class. Pros: nodes don't need to know about colors and then they wouldn't, and draw and the guy could use the same infrastructure with different color tables. Cons: more of a pain to maintain as such color tables may need to be updated (maybe in multiple places) if a new node is introduced. Overall I still think this is the best attack.

[samwaseda]

the core infrastructure shout have no idea the gui exists

And for this very reason I find it actually surprising that there are colours attached to the nodes in the first place. For me a helpful distinction would be whether there are more nodes underneath, which in the case of pyiron_workflow coincides with node vs. macro, but I guess we can also do the same distinction by whether it has children or not?

One way or other, I think there should be a cleaner interface between node and draw. Ideally pyiron_workflow should be able to export an entire graph first in a simpler format like dict, which is taken over by draw.py. And from my point of view, it should be entirely up to draw.py to figure out how to plot it.

For this PR, if it's all about darkness, we can maybe define something similar to lighten_hex_color for now on the GUI side?

[Tara-Lakshmipathy]

Is there an attribute which gives me the type of the node - function, composite etc.? I feel this would be useful in a variety of situations. On the S-Bahn for the next 50 minutes, but can't wait that long to check for myself 😆.

EDIT: nvm I saw in the draw module that isinstance() is used. But I think having an attribute that returns the type of node directly would be a neat convenience feature for a bunch of front-end applications, especially when an external tool wants to interface with 'pyiron_workflow' without knowing about the internal python classes used in the workflow code. Also convenient for filling metadata fields on publishing platforms.

[liamhuber]

And for this very reason I find it actually surprising that there are colours attached to the nodes in the first place.

One way or other, I think there should be a cleaner interface between node and draw

Yes, this is absolutely just bad/lazy abstraction on my part. This information needs to reside inside the pyiron_workflow package, because it's important to me that the base package ships with the ability to provide a non-code representation (i.e. draw), but the colour data and logic absolutely should all be inside the draw module.

For me a helpful distinction would be whether there are more nodes underneath, which in the case of pyiron_workflow coincides with node vs. macro, but I guess we can also do the same distinction by whether it has children or not?

Is there an attribute which gives me the type of the node - function, composite etc.?

What you're looking for is just the class inheritance -- you just want isinstance here and a prioritized dict of type[Node]: str to match classes to colors.

I agree the main two are Function and Macro. Beyond this, I also think it's useful for Transformer (the parent class for dataclass nodes, list-to-output, input-to-list, etc), For, and If all to get unique colors. I'm not clairvoyant, but -- unless we introduce a specific While node instead of just constructing it as a macro -- that set should provide colors for the foreseeable future.

Ideally pyiron_workflow should be able to export an entire graph first in a simpler format like dict

This exists for Composite nodes (macros and workflows) -- Composite.graph_as_dict! draw was one of the first things I implemented, and graph_as_dict was a convenience method added much later (you can go look, it is really simply iterating over things the nodes already provide easy access to), thus in the spirit of "if it's working, don't touch it", I never tried to leverage the new convenience in draw. In the spirit of "refactor, refactor, refactor", if you want to leverage this in draw, or find a convenient way to pull the method from Composite up to Node and use that, I'm on board.

[liamhuber]

But I think having an attribute that returns the type of node directly...

I disagree -- there is a straightforward pythonic way of finding what type of node we have and it is type(node) or isinstance(node, ThingIAmCheckingAgainst). This is already super straightforward and I don't think we should reinvent the wheel/create more

...would be a neat convenience feature for a bunch of front-end applications, especially when an external tool wants to interface with 'pyiron_workflow' without knowing about the internal python classes used in the workflow code.

Here I agree, but the way I suggest to do it is as I mentioned earlier: pyiron_workflow.draw, which is embedded right in the module and knows all the core classes, should know how to map those core classes to nice default colours. This is convenient without stepping on python's toes, and should cover 90%+ of use cases. If someone wants a non-standard colour for some particular node, nothing stops them from doing that, and whatever pyiron_workflow.node.get_color(node: Node) looks like, it should be flexible enough that they can extend it. E.g. perhaps it's like pyiron_workflow.draw.get_color(node: Node, color_map: Optional[dict]: None) and we can also provide pyiron_workflow.draw.color_map: dict[type[Node], str] that they could import, extend/override, then pass to get_color.

So I'm for convenience, but against alternate routes for standard language stuff.

[Tara-Lakshmipathy]

If we have get_color as opposed to the color we have now, then I would be in agreement. And since this pull request is about colors, I don't want to deviate from that topic too much. But I agree with @samwaseda mentioned earlier. I was surprised to find color in these files. Initially I expected to find it in draw.py as well. So if we all agree to attack this by getting rid colors from the nodes themselves, then count me in.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.15% (target: -1.00%)	✅ 50.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (d8728f1)	3371	3088	91.60%
Head commit (0087438)	3383 (+12)	3094 (+6)	91.46% (-0.15%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#486)	12	6	50.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}