Skip to content

Conversation

@kamurani
Copy link
Contributor

@kamurani kamurani commented Jul 22, 2022

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

  • Added a note about the modification or contribution to the ./CHANGELOG.md file (if applicable)
  • Added appropriate unit test functions in the ./graphein/tests/* directories (if applicable)
  • Modify documentation in the corresponding Jupyter Notebook under ./notebooks/ (if applicable)
  • Ran 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)
  • Checked for style issues by running black . and isort .

@kamurani
Copy link
Contributor Author

kamurani commented Jul 22, 2022

Testing asteroid plot, scaling by RSA and colour by hydrophobicity

fig1 = asteroid_plot(
    g=g,
    node_id=node,
    colour_nodes_by="hydrophobicity",
    size_nodes_by='rsa',
    node_size_multiplier=80
)

image

@a-r-j a-r-j marked this pull request as ready for review July 22, 2022 09:10
@kamurani
Copy link
Contributor Author

@a-r-j any changes you want me to make?

@a-r-j
Copy link
Owner

a-r-j commented Jul 26, 2022

@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!

return hmap[res]

node_colours = []
for n in subgraph.nodes():
Copy link
Owner

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)
...

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(
Copy link
Owner

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?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 2, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

No Coverage information No Coverage information
10.5% 10.5% Duplication

@a-r-j
Copy link
Owner

a-r-j commented Oct 23, 2022

.Hey @kamurani any chance you can approve this PR so we can get this merged in? Thanks!!

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

No Coverage information No Coverage information
5.8% 5.8% Duplication

@a-r-j a-r-j mentioned this pull request Jan 25, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

No Coverage information No Coverage information
1.7% 1.7% Duplication

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

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.

2 participants