Skip to content

Conversation

@atravitz
Copy link
Contributor

continuation of #91 - making a new PR to not spam everyone tagged there.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

Colab 👈 Launch a Colab session on branch cofactors_cookbook

@atravitz atravitz self-assigned this Jan 20, 2026
@atravitz atravitz requested a review from jthorton January 20, 2026 18:15
@atravitz atravitz marked this pull request as ready for review January 20, 2026 18:15
@@ -0,0 +1,372 @@
{
Copy link
Contributor

@jthorton jthorton Jan 22, 2026

Choose a reason for hiding this comment

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

Maybe its worth adding a note that we will be using pre-charged ligands here and pointing to the charge-molecules CLI docs for more info on how to generate these files?

I would also add a warning box highlighting that supplying cofactors this way means they are parameterised using the small molecule force field, I am not sure if its also worth highlighting how a different force field could be used or if that would confuse things.


Reply via ReviewNB

@@ -0,0 +1,372 @@
{
Copy link
Contributor

@jthorton jthorton Jan 22, 2026

Choose a reason for hiding this comment

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

SDf -> SDF?


Reply via ReviewNB

@@ -0,0 +1,372 @@
{
Copy link
Contributor

@jthorton jthorton Jan 22, 2026

Choose a reason for hiding this comment

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

Line #4.    offmol.assign_partial_charges('am1bcc')

This is no longer needed as the charges are in the SDF file.


Reply via ReviewNB

Copy link
Contributor

@jthorton jthorton left a comment

Choose a reason for hiding this comment

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

A few minor changes, but overall this looks great, thanks for finishing it after many years!

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.

5 participants