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

Fragment filter #270

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fragment filter #270

wants to merge 1 commit into from

Conversation

jduerholt
Copy link
Contributor

This PR adds a method that can be used to get only the contained fragments for a list of smiles. This can then be used to intitialize the Fragments object only with the needed fragments. One could also think about adding a method which returns only the fragments with variance in it.

@jduerholt
Copy link
Contributor Author

@simonsung06 : can you have a look?

@simonsung06
Copy link
Contributor

Sorry for the delay. I left a comment on the test. Can you see it?

@jduerholt
Copy link
Contributor Author

No problem, but I cannot see any comment.

Copy link
Contributor

@simonsung06 simonsung06 left a comment

Choose a reason for hiding this comment

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

I like the feature. minor comment

"N[C@](C)(F)C(=O)O",
]
)
mf.get_contained_fragments(smiles)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make the test have an assert for an expected result too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can do this, could be better. I will add this later.

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