-
-
Notifications
You must be signed in to change notification settings - Fork 366
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
Add initial graph layout algorithm #398
Conversation
datashader/layout.py
Outdated
|
||
# This comes from the sparse FR layout in NetworkX | ||
A = nx.to_scipy_sparse_matrix(graph, dtype='f') | ||
nnodes, _ = A.shape |
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.
Is this just a temporary implementation for testing? We can't depend on networkx in general.
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.
This was meant as temporary until I could switch over to using dataframes for input. The original implementation relied on NetworkX.
datashader/layout.py
Outdated
nodes. | ||
dim : int | ||
Coordinate dimensions of each node. | ||
|
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.
If this is a ParameterizedFunction, it should have parameters, i.e. declare iterations = param.Integer(10,doc="Number of iterations")
at the class level, with __call__
set up as other ParameterizedFunctions are (no explicit named arguments like that) and no docstring like this.
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.
Thanks. I updated the class.
datashader/layout.py
Outdated
# this is the largest step allowed in the dynamics. | ||
t = 0.1 | ||
|
||
# simple cooling scheme. |
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.
Should the meat of this function be in a separate Numba-ized function?
We removed the NetworkX requirement and can now take two dataframes (nodes and edges) as input, like the bundling module.
@jbednar Thanks for the feedback. The initial algorithm is now fully parameterized, uses only dataframes for both input and output, and uses Numba as appropriate. I'm going to merge this so that I can work on a simple demo and then create a new dev release. |
Looks good, thanks! |
No description provided.