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

adding mol name labels from ofe-name to mapping vis #241

Merged
merged 4 commits into from
Nov 22, 2023

Conversation

RiesBen
Copy link
Contributor

@RiesBen RiesBen commented Sep 27, 2023

This pull request adds one nice small features to gufe mapping visualization:

  • labeling mapped molecules with their ofe-name
    image

@RiesBen RiesBen self-assigned this Sep 27, 2023
@RiesBen RiesBen added the enhancement New feature or request label Sep 27, 2023
@RiesBen RiesBen linked an issue Sep 27, 2023 that may be closed by this pull request
@RiesBen RiesBen enabled auto-merge September 27, 2023 12:05
@pep8speaks
Copy link

pep8speaks commented Sep 27, 2023

Hello @RiesBen! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2023-11-20 12:26:56 UTC

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (af0d3bd) 99.20% compared to head (532f988) 99.20%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #241   +/-   ##
=======================================
  Coverage   99.20%   99.20%           
=======================================
  Files          36       36           
  Lines        1892     1893    +1     
=======================================
+ Hits         1877     1878    +1     
  Misses         15       15           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mikemhenry mikemhenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the documentation to add a note that ofe-name is used to label the molecules? Also is this something that users might want to just turn off? Could the ofe-name be really ugly or is that completely in the users hands?

@richardjgowers richardjgowers self-assigned this Oct 2, 2023
@richardjgowers
Copy link
Contributor

@mikemhenry I think there's also some magic where Mol.GetProp('_Name') (the semi canonical way of labelling a name in rdkit) is converted to ofe-name (and vice versa)

@mikemhenry mikemhenry self-requested a review October 2, 2023 21:56
@mikemhenry mikemhenry dismissed their stale review October 2, 2023 21:57

We have magic, so no documentation needed

@richardjgowers richardjgowers force-pushed the adding_mol_name_labels_to_mapping_vis_only branch from 3c9ce0f to 532f988 Compare November 20, 2023 12:26
@RiesBen RiesBen merged commit 7d10127 into main Nov 22, 2023
15 checks passed
@RiesBen RiesBen deleted the adding_mol_name_labels_to_mapping_vis_only branch November 22, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adding mol name to mapping figure?
4 participants