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

Expose sire.convert functionality #9

Merged
merged 27 commits into from Feb 23, 2023
Merged

Expose sire.convert functionality #9

merged 27 commits into from Feb 23, 2023

Conversation

lohedges
Copy link
Contributor

This PR exposes the new functionality from sire.convert through the BioSimSpace.Convert sub-package. I have also wrapped functionality to create molecules from SMILES strings, and have exposed this on all wrapped Sire objects, i.e. so you can do things like system.smiles() to get a list of SMILES strings for all molecules in the system.

The PR also resolves some minor formatting issues and typos in the documentation, fixes issue #8, and adds support for reading remote trajectory files, as discussed here.

Note that this PR currently requires functionality from the feature_rdkit2 branch of Sire, so I will cancel CI until this is ready. (We will also need a new Sire release, so this PR can be built against that version upwards.) Alternatively I have workarounds that will allow it to work with the current Sire devel, which could be removed at a later date. (Although I would need to disable certain functionality on Windows.)

Checklist

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have added a test for any new functionality in this pull request: [y]
  • I confirm that I have added documentation (e.g. a new tutorial page or detailed guide) for any new functionality in this pull request: [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

@chryswoods

chryswoods
chryswoods previously approved these changes Feb 22, 2023
Copy link
Contributor

@chryswoods chryswoods left a comment

Choose a reason for hiding this comment

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

This all looks good :-)

@lohedges
Copy link
Contributor Author

Thanks. I'll wait until the Sire devel build is done, then merge. The only thing that I had thought of was adding convenience functions to some of the wrapped Sire objects so that you can do .toRDKit(), etc. Perhaps I can just bind these automatically at import time by checking what's available in BioSimSpace.Convert.

Oh, does you merge include a release bump, since this PR won't work with the original RDKit feature? It should pull in the most recent package, but who knows. Once there is a new release I can update the Sire requirement.

@chryswoods
Copy link
Contributor

No, sorry, I didn't do a version bump. I'll remove the old packages from anaconda.org though, as I want to try and keep it so that we only have one latest devel package available. I agree it may lead to some breakage for people using those packages, but that is the price of living on the bleeding edge ;-)

@lohedges lohedges merged commit 5fa7e96 into devel Feb 23, 2023
@lohedges lohedges deleted the feature_convert branch February 23, 2023 10:10
@lohedges lohedges added the enhancement New feature or request label Mar 16, 2023
lohedges pushed a commit that referenced this pull request Jun 26, 2023
Fix OpenMM restraints. Checks failing because of new `pint` version (fixed in #13).
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.

None yet

2 participants