Skip to content

Implement a GLGraphItem according to #807#1721

Merged
j9ac9k merged 3 commits intopyqtgraph:masterfrom
j9ac9k:test-pr807
Sep 8, 2021
Merged

Implement a GLGraphItem according to #807#1721
j9ac9k merged 3 commits intopyqtgraph:masterfrom
j9ac9k:test-pr807

Conversation

@j9ac9k
Copy link
Copy Markdown
Member

@j9ac9k j9ac9k commented Apr 17, 2021

PR #807 aims to add a GLGraphItem widget. I think this is a good idea, and the implementation looks good at a glance. What the PR was missing is an example, and updates to the documentation to reference it.

It appears the fork that the PR was created from has since been deleted, so I'm having to create a new PR w/ the changes.

Right now, the example is super-basic. I was mostly looking to get something working, I'll think up something more interesting before merging; also would like to open this up for input from others.

While the PR is functional; work should be done here to support taking in data structures from the output of networkx library; and to provide a way to conditionally style the nodes/edges.

Some things I think the API should be able to handle

  • Plot data structures that are the output of networkx.drawing.layout.<whatever>(G, ndim={2, 3}) for position data (this involves a dictionary keyed with nodeID, and the values are numpy arrays representing the position for each dimension
  • Take edge information from G.edges which gives back a MultiEdgeView object

7/25 update: So getting style information from the MultiEdgeView object, is straight forward, but drawing those details is another story. Specifically the edge color, and line width is going to be a bit tricky to implement. I'm considering skipping handling any edge-specific features, and just make the color consistent everywhere. Note, I think it will be straight forward to handle node-specific styling features.

7/30 update: Drawing edge specific features such as line width and color is definitely outside of my skillset with opengl. I could probably figure out something, but it would be a mountain of code and probably subject to pretty awful breakage, so I think I'm going to leave that functionality out for the time being.

@NilsNemitz
Copy link
Copy Markdown
Contributor

This seems like it would be nice to have as part of the OpenGL functions!

One thing I noticed is that this gives GLScatterPlotItem an __init__() keyword argument parentItem that ScatterPlotItem does not accept (as far as I can tell).
Is it possible to implement setting the parent in the same way as in (non-OpenGL) GraphItem, which initializes ScatterPlotItem first and then calls its inherited setParentItem() method? There is already a setParentItem() in GLGraphicsItem.
Otherwise we might add parentItem to the keyword list of (non-OpenGL) ScatterPlotItem and have GraphItem initialize it with the correct parent right away.

It might be a good goal to try and keep the OpenGL / non-OpenGL items as similar as (conveniently) possible. :)

@NilsNemitz NilsNemitz changed the title Implement #807 Implement a GLGraphItem according to #807 Apr 17, 2021
@j9ac9k j9ac9k linked an issue Jul 15, 2021 that may be closed by this pull request
@j9ac9k
Copy link
Copy Markdown
Member Author

j9ac9k commented Jul 24, 2021

Thanks @NilsNemitz for the feedback, good suggestion, incorporated!

So this effectively works right now, the main question I have is is the API sufficient here?

To test this out, I'm going to spend some time trying to plot some of the items from the network-x gallery ... some of their position methods/functionality can output positions in 3D. Trying to replicate some of their plots that way should hopefully identify current pain points well.

@j9ac9k
Copy link
Copy Markdown
Member Author

j9ac9k commented Jul 24, 2021

I saw elsewhere in the library we are able to plot different colors in one go using

                if isinstance(self.color, np.ndarray):
                    glEnableClientState(GL_COLOR_ARRAY)
                    glColorPointerf(self.color)

but is there analogous to multiple line widths so I am not limited to a single width in one pass like I am when I call

                glLineWidth(self.edgeWidth)

@j9ac9k
Copy link
Copy Markdown
Member Author

j9ac9k commented Jul 25, 2021

Updated OP, handling multi-width lines, and handling different line colors is not something I know how to handle in OpenGL (for the record, I'm sure this can be done with OpenGL, I'm saying I'm not sure how to implement it without going back to the drawing board).

@j9ac9k j9ac9k marked this pull request as ready for review July 31, 2021 03:54
@j9ac9k
Copy link
Copy Markdown
Member Author

j9ac9k commented Jul 31, 2021

Marked as ready for review, only real change I see I want to make here is a more interesting example...

If anyone knows a relatively straight forward way to draw the edges with different widths or draw edges with specific colors, please share.

Copy link
Copy Markdown
Member

@ixjlyons ixjlyons left a comment

Choose a reason for hiding this comment

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

Adding this to the docs would be good.

Otherwise looks pretty good to me, mostly nit-pick stuff.

As for the multiple-edge-width thing, aside from drawing the lines individually, I think you'd basically have to generate pairs of triangles to achieve that effect efficiently. I'm pretty sure you could write a shader so the conversion from node positions isn't done on the CPU (in case the user dynamically updates node positions). Seems like something vispy might be good at :)

* QColor
* an array [red, green blue, alpha]
* None (to disable edge drawing)
* [not available yet] a record array of length M
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd remove this unless it's a planned enhancement

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this is a good feature to have; it's outside of my skill-set with openGL regarding how to implement. I'll leave this comment as un-resolved, but leave the comment in there in case anyone wants to come along and suggest an implementation.

nodeSize (N,) array of floats specifying spot sizes
OR a single value to apply to all spots. (overwrites size)
``**opts`` All other keyword arguments are given to
:func:`GLScatterPlotItem.setData() <pyqtgraph.GLScatterPlotItem.setData>`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cross-reference should be :meth:`GLScatterPlotItem.setData() <pyqtgraph.opengl.GLScatterPlotItem.setData>`

@j9ac9k j9ac9k force-pushed the test-pr807 branch 2 times, most recently from 4869809 to 3e4fdf8 Compare August 2, 2021 05:24
@j9ac9k
Copy link
Copy Markdown
Member Author

j9ac9k commented Aug 2, 2021

@ixjlyons Thanks for the feedback, I incorporated just about everything I think... things do look better; also it forced me to think about setting some of the parameters in a more robust fashion.

Anyway one thing that I'm scratching my head at is the rendered docs:

https://pyqtgraph--1721.org.readthedocs.build/en/1721/3dgraphics/glgraphitem.html?highlight=glgraphitem#pyqtgraph.opengl.GLGraphItem

I noticed that they don't have that nice rendered table the sphinx docstring style seems to generate, is this a known issue or is there something I missed?

@ixjlyons
Copy link
Copy Markdown
Member

ixjlyons commented Aug 2, 2021

"Sphinx style" for argument lists are field lists. The numpy style generates similar output, just using a slightly different syntax. The argument tables currently used in pyqtgraph are really just RST tables. So the theme could probably be modified to make field lists appear a little more like tables, but note how numpy's docs have the same field list look.

@j9ac9k
Copy link
Copy Markdown
Member Author

j9ac9k commented Aug 2, 2021

"Sphinx style" for argument lists are field lists. The numpy style generates similar output, just using a slightly different syntax. The argument tables currently used in pyqtgraph are really just RST tables. So the theme could probably be modified to make field lists appear a little more like tables, but note how numpy's docs have the same field list look.

Ahh good point on how our docs are currently RST tables; ... just wanted to make sure I didn't mess something up. 👍🏻

@j9ac9k j9ac9k force-pushed the test-pr807 branch 3 times, most recently from 83cfa63 to cd9c95d Compare August 4, 2021 05:35
Copy link
Copy Markdown
Member Author

@j9ac9k j9ac9k left a comment

Choose a reason for hiding this comment

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

Accidentally hit started a review button, and not seeing how to cancel out...

@j9ac9k j9ac9k merged commit 446a2c3 into pyqtgraph:master Sep 8, 2021
@pijyoi
Copy link
Copy Markdown
Contributor

pijyoi commented Jul 31, 2024

Regarding the edge-specific color feature, it wouldn't be difficult to implement it after #3109 gets merged.
GLGraphItem has been re-implemented using GLLinePlotItem lines mode, and GLLinePlotItem does accept an array of colors.

As an example, GLAxisItem was also re-implemented using GLLinePlotItem and does this to give the 3 axis lines their own color.

@pijyoi pijyoi mentioned this pull request Feb 24, 2025
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.

GraphItem in 3D

4 participants