-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
Graph plots #197
base: master
Are you sure you want to change the base?
Graph plots #197
Conversation
@a-r-j any changes you want me to make? |
Hey, sorry - busy period. A few minor comments but otherwise LGTM. Thanks for the contributions! |
graphein/protein/visualisation.py
Outdated
return hmap[res] | ||
|
||
node_colours = [] | ||
for n in subgraph.nodes(): |
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.
I believe you can replace this nested loop with:
for n, d in subgraph.nodes(data=True)
...
…ith more features
…ding which scale to use; or just return vector of values for all 5 scales.
TODO: Functino that gets ``min`` and ``max`` values in a graph for a given feature so that we can scale / offset to > 0 | ||
TODO: should feature `distance` actually contain the site itself i.e. in the string? | ||
""" | ||
def _node_feature_func( |
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 is really elegant work - thanks a lot!
Having taken a closer look at the code I wonder if we can be even smarter here. Given the rest of Graphein is quite functional in style, we could perhaps refactor out the sizing functions into atomic functions & then allow users to pass in either a function of their own or a string which would dispatch one the lambdas/more complicated functions you've implemented here. I think this provides the most flexibility whilst supporting all the great features you've added here. What do you think?
SonarCloud Quality Gate failed. 0 Bugs No Coverage information |
SonarCloud Quality Gate failed. 0 Bugs No Coverage information |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Reference Issues/PRs
couldn't find an issue that references this
What does this implement/fix? Explain your changes
add support for colouring / sizing asteroid plot nodes by different features. Extra functions can be added for more features if it is known how to reference the node data e.g.
g.nodes(data=True)[k]['rsa']
change colourscheme to viridis (can revert this if it's unwanted)
What testing did you do to verify the changes in this PR?
Plotted example graphs.
Pull Request Checklist
./CHANGELOG.md
file (if applicable)./graphein/tests/*
directories (if applicable)./notebooks/
(if applicable)python -m py.test tests/
and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g.,python -m py.test tests/protein/test_graphs.py
)black .
andisort .