Those changes seem reasonable to me. Is there a specific omm version where this change occurs? Could modify the requirements to specify the versions that are compatible with this change as well.

This is probably a majorly breaking change - the PR making that change was merged months ago but it has not made its way into a release. The tests using OpenMM are likely all skipped via the @needs_openmm decorator, which probably also toggles off all OpenMM-related functionality.

I’m not sure about the openmm version where these changes occur, but the requirement would need to be updated. It’s also fine with me if you don’t want to merge this. I made this change in our code base, but was still getting a warning about simtk, until I realized parmed (which we use) is also importing simtk.openmm.

It might also be possible to make this change in a more backwards compatible way. I think the goal would be that import parmed doesn’t cause the warning about simtk. This would require something like:

try:
    import openmm as mm
except ImportError:
    import simtk.openmm as mm

This change would only need to be made in the main code path, not in all the tests.

Does this seem like a better approach?

I don't have the final say but that does seem like a better approach to me - since the change (whenever it goes into a release, I don't know when this is going to happen) is limited to the import path and not the rest of the API, people are likely going to be using a mix of older and newer versions for a while.

I had at one point tried to centralize the OpenMM imports and use some tricks to fall back on simtk if openmm wasn't available. It wasn't worth the hassle for an optional dependency that is completely free.

I was holding off on making this change until OpenMM cut a release with this switch (which it hasn't yet).

The unit tests do currently exercise OpenMM, but the reason they aren't all failing now is because, as @mattwthompson pointed out, the testing framework identifies OpenMM as not being available and just skips them (which is why the test coverage of this PR would be substantially lower than what the whole test suite currently achieves).

I'm going to wait on merging this until after OpenMM has a release so I have the freedom to cut a new ParmEd release before OpenMM does theirs.

An alternative is to try and convince @peastman to create a transitional stub library that is pip/conda-installable that literally just imports stuff from the simtk namespace into the package (named openmm) as a way of making older versions forwards-compatible through the name change.

OTOH, hoisting deprecation warnings for a transitional period doesn't seem that horrible to me (plenty of popular packages I work with do it).

We haven't scheduled the 7.6 release yet, which is where the new namespace will be introduced. Until it's released, I would recommend sticking with simtk. The worst that happens is that if someone has installed a development version of OpenMM, they get a deprecation warning.

We haven't scheduled the 7.6 release yet, which is where the new namespace will be introduced.

This strikes me as a very strong justification for cutting an 8.0, for my 2c.

The OpenMM feedstock maintainers have released a 7.6 beta which can now be used to test the new namespace:

mamba install -c conda-forge/label/openmm_rc -c conda-forge "openmm=7.6beta"

I think this can now be merged?

Is there anything still blocking this PR? It needs to be merged before I can fix the AMOEBA code.

Just a reminded that I'm still waiting on this!

@swails: Are there other developers with commit access that could help, or would you be willing to give someone from the OpenMM/OpenFF community commit access to help maintain ParmEd?

There's another PR that fixes ParmEd with OpenMM 7.6 (#1199) that still maintains backwards compatibility with older versions (for now).

Is that sufficient for the amoeba work to begin? It pins omm 7.6 in the test environment now.

I don't think anyone else has commit access, but I'm open to granting it to someone that's interested.

That one would work too. Thanks!

Resolved in #1199