Skip to content
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

Asteroid plot edges #170

Merged
merged 7 commits into from
May 13, 2022
Merged

Asteroid plot edges #170

merged 7 commits into from
May 13, 2022

Conversation

avivko
Copy link
Contributor

@avivko avivko commented May 13, 2022

Reference Issues/PRs

Fixes #167
Note: hoovering over the edges does not show the edge kind. @a-r-j Feel free to have a look at it!

What does this implement/fix? Explain your changes

  • Adds the options to use show_edges without getting an error.
  • Also now you could get rgba strings returned from the colour_edges function
  • Also you can now set an alpha value for the edges through colour_edges (e.g. in the asteroid plot)

What testing did you do to verify the changes in this PR?

Ran it on JN and got a plot with edges without an error (attached)
newplot (1)

@a-r-j
Copy link
Owner

a-r-j commented May 13, 2022

Thanks for the PR!

Ah, yes, the line hover doesn't work on 2D scatters in Plotly AFAIK. If you're really desparate for it, one solution I've seen is to add transparent nodes on the midpoint of each line. https://stackoverflow.com/questions/46037897/line-hover-text-in-plotly

@codecov-commenter
Copy link

codecov-commenter commented May 13, 2022

Codecov Report

Merging #170 (9be13eb) into master (8123f42) will increase coverage by 8.69%.
The diff coverage is 56.09%.

@@            Coverage Diff             @@
##           master     #170      +/-   ##
==========================================
+ Coverage   40.27%   48.96%   +8.69%     
==========================================
  Files          48       74      +26     
  Lines        2811     4254    +1443     
==========================================
+ Hits         1132     2083     +951     
- Misses       1679     2171     +492     
Impacted Files Coverage Δ
graphein/grn/parse_trrust.py 37.77% <ø> (ø)
graphein/ml/diffusion.py 0.00% <0.00%> (ø)
graphein/ppi/edges.py 100.00% <ø> (ø)
graphein/ppi/graph_metadata.py 0.00% <ø> (ø)
graphein/ppi/graphs.py 54.34% <ø> (ø)
graphein/ppi/parse_biogrid.py 75.00% <ø> (ø)
graphein/ppi/visualisation.py 0.00% <0.00%> (ø)
graphein/protein/analysis.py 0.00% <0.00%> (ø)
graphein/protein/features/sequence/sequence.py 71.42% <0.00%> (+2.67%) ⬆️
graphein/protein/features/sequence/utils.py 28.00% <0.00%> (+3.00%) ⬆️
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91c5f59...9be13eb. Read the comment docs.

@a-r-j
Copy link
Owner

a-r-j commented May 13, 2022

@avivko Happy to merge unless you want to try your hand at the edge labelling?

@avivko
Copy link
Contributor Author

avivko commented May 13, 2022

Thanks for the PR!

Ah, yes, the line hover doesn't work on 2D scatters in Plotly AFAIK. If you're really desparate for it, one solution I've seen is to add transparent nodes on the midpoint of each line. https://stackoverflow.com/questions/46037897/line-hover-text-in-plotly

Nah, it's fine. but I decided to add an option to display a legend that groups the edges by kind. This way you can click certain edge kinds away, which is neat/useful in my opinion. I'll push it in a jiffy.
newplot (2)
newplot (3)

@avivko
Copy link
Contributor Author

avivko commented May 13, 2022

@a-r-j Also, I see that one of your tests fails because of the ye old broken conda environment file.
I've made my own, simplified version of it that runs w/o resulting in conflicts, in case you want to give it a try:

cat graphein-gpu.yml 
name: graphein-gpu
channels:
  - salilab
  - schrodinger
  - pytorch
  - pyg
  - conda-forge
  - pytorch3d
  - defaults
dependencies:
  - python=3.8
  - pip
  - jupyter
  - ipywidgets
  - dssp
  - pymol
  - vmd-python
  - pytorch
  - torchvision
  - cudatoolkit=11.3
  - pytorch3d
  - pip:
    - graphein[all]
    - biovec

@a-r-j
Copy link
Owner

a-r-j commented May 13, 2022

Great - that's a really nice solution.

Re the conda env, I'll check it out in another PR.

Thanks for the contribution! Appreciate the effort :)

@a-r-j a-r-j merged commit 1c87afd into a-r-j:master May 13, 2022
@sonarcloud
Copy link

sonarcloud bot commented May 13, 2022

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 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

This pull request was closed.
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.

asteroid_plot: show_edges
3 participants