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

deal with openmm 7.6 package unwrapping #528

Merged
merged 2 commits into from
Nov 15, 2021

Conversation

sroet
Copy link
Contributor

@sroet sroet commented Nov 14, 2021

Description

importing OpenMMTools raises warinings about importing openmm from simtk, which make OpenPathSampling's notebook tests fail (unexpected warning in the stderr output)

This updates all imports to the new standard.

I went through grep -r -n "simtk" --include="*.py" -l and updated all references to simtk. For the doc-tests I decided to only include the current advised method of importing openmm. For all real code, I wrapped the new method in a try/except clause that drops back to the old style imports.

Todos

  • Implement feature / fix bug
  • [NA] Add tests
  • [NA] Update documentation as needed
  • Update changelogNotable points that this PR has either accomplished or will accomplish.

Status

  • Ready to go

@jchodera
Copy link
Member

Thanks so much for updating this in a backwards-compatible manner!

@ijpulidos can you review and merge when able?

@sroet
Copy link
Contributor Author

sroet commented Nov 14, 2021

test failure is identical to master. (and is for an unsupported python, based on NEP 29)

@jchodera
Copy link
Member

Thanks! And thanks for pointing out NEP 29---I hadn't seen that before, but we will try to standardize around that!

Copy link
Contributor

@ijpulidos ijpulidos left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

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.

3 participants