-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Improved Graph element #2145
Improved Graph element #2145
Conversation
162828a
to
d5b9651
Compare
d5b9651
to
a7eeaf8
Compare
holoviews/element/graphs.py
Outdated
N = len(nodes) | ||
circ = np.pi/N*np.arange(N)*2 | ||
x = np.cos(circ) | ||
y = np.sin(circ) | ||
return (x, y, nodes) | ||
|
||
|
||
def connect_edges_pd(graph): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move some of these functions to element.util?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sounds good.
@@ -411,6 +412,59 @@ def map_colors(arr, crange, cmap, hex=True): | |||
return arr | |||
|
|||
|
|||
def mplcmap_to_palette(cmap, ncolors=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the right place for these utilities!
tests/testbokehgraphs.py
Outdated
g = self.graph4.opts(plot=dict(edge_color_index='Weight'), | ||
style=dict(edge_cmap=['#FFFFFF', '#000000'])) | ||
plot = bokeh_renderer.get_plot(g) | ||
print(plot.handles) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stray print.
Looks good! Other than some very minor comments I posted above, I am happy for you to go ahead and merge once you think it is ready. |
Yay, even got a gain in coverage. Thanks for the review, merging. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Since I started writing the Chord (#2138) and TriMesh (#2143) elements, it's become clear that the Graph element is quite useful as a baseclass for other elements. However currently it isn't as solid, optimized and tested as I'd like it to be, so rather than mixing up a bunch of changes into the
Chord
andTriMesh
PRs I've decided to first build onGraph
, ensuring that it is well tested and solid at it's core.This will improve Graphs in a number of ways:
Graph Element Improvements
Graph Plot improvements
GraphPlot
implementations now support anedge_color_index
allowing them to be colored both by categorical attributes (e.g. by their source node) or by a value (e.g. a weight value).