-
-
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
Fix and enhance geometric node features #305
Conversation
for more information, see https://pre-commit.ci
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #305 +/- ##
==========================================
+ Coverage 40.27% 44.10% +3.83%
==========================================
Files 48 113 +65
Lines 2811 7817 +5006
==========================================
+ Hits 1132 3448 +2316
- Misses 1679 4369 +2690
☔ View full report in Codecov by Sentry. |
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.
LGTM! Thanks for another great contribution :) Can you please add a note to the changelog (also for #301)?
Without this, `vec`s are of type object, which for example breks the smooth conversion to torch tensors.
for more information, see https://pre-commit.ci
graphein/ml/visualisation.py
Outdated
@@ -115,13 +116,17 @@ def plot_pyg_data( | |||
d["coords"] = x.coords[i] | |||
if node_colour_tensor is not None: | |||
d["colour"] = float(node_colour_tensor[i]) | |||
if hasattr(x, "c_beta_vector"): |
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 a really nice idea. I think it would be better to generalise this and provide these as default arguments.
E.g.:
for node_vec in node_vector_features:
if hasattr(x, node_vec):
d[node_feat] = getattr(x, node_feat)[i]
graphein/ml/visualisation.py
Outdated
@@ -135,3 +140,12 @@ def plot_pyg_data( | |||
colour_nodes_by if node_colour_tensor is None else "colour", | |||
colour_edges_by if edge_colour_tensor is None else "colour", | |||
) | |||
if hasattr(x, "c_beta_vector"): |
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.
As above
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Hi, @a-r-j! Can you take another look at this? It would be nice if you could merge this. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Hey @anton-bushuiev sorry for being slow on this. Recently started a new job & this PR slipped through the cracks. Appreciate the contribution, as always. |
Reference Issues/PRs
rgroup_df=compute_rgroup_dataframe(remove_insertions(raw_pdb_df))
#304H:CYS:104
in3SE8
).object
, which breaks for example their smooth conversion to PyTorch tensors, when converting to PyG data.add_k_nn_edges
for the case when some residues were dropped before (e.g. when some alt_locs are removed). The code ofadd_k_nn_edges
assumes the dataframe to have continuous index which is not true if some residues are dropped. For example the following line constructs continuousoutgoing
which are then used to index dataframe:graphein/graphein/protein/edges/distance.py
Line 1127 in 371ce9a
I have also added the functionality to test whether a warning is raised by
loguru
according to Delgan/loguru#59.What does this implement/fix? Explain your changes
What testing did you do to verify the changes in this PR?
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 .