search

openforcefield / openff-toolkit

The Open Forcefield Toolkit provides implementations of the SMIRNOFF format, parameterization engine, and other tools. Documentation available at http://open-forcefield-toolkit.readthedocs.io
http://openforcefield.org
MIT License
301 stars 88 forks source link

Drop py311 from conda tests #1873

Closed j-wags closed 2 months ago

j-wags commented 2 months ago

OpenMMForceFields requires AmberTools <23, but AmberTools <=22 doesn't support python 3.11

mattwthompson commented 2 months ago

I don't think openmmforcefields is used in these tests?

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.70%. Comparing base (f282282) to head (3dc9f7f). Report is 1 commits behind head on main.

Additional details and impacted files
mattwthompson commented 2 months ago

Oh, I see, it's because of this trick: https://github.com/openforcefield/openff-toolkit/blob/f282282c6c5fb14df4c479cb7c689984bf3952f0/devtools/conda-envs/conda.yaml#L5-L6

I think this should be changed until unless openmmforcefields is a base dependency of the toolkit

j-wags commented 2 months ago

I think this should be changed until openmmforcefields is a base dependency of the toolkit

I'd like to interpret "this should be changed..." as "the changes proposed in this PR should remain until...", but I'm not sure that's what you meant.

These conda tests are the "whole kit and kaboodle", including external examples which require OMMFFs. I don't currently plan for OMMFFs to become a dependency of openff-toolkit or openff-toolkit-base.

j-wags commented 2 months ago

And the immediate issue is that we have our conda tests running with py3.10 and 3.11, the latter of which will be failing until the deployment issue is resolved.

mattwthompson commented 2 months ago

I have no idea how my hands threw an "until" in there, "unless" fits better (I don't think it should become a dependency!). Sorry for the confusion

These conda tests are the "whole kit and kaboodle"

Right, it brings in everything needed for base functionality, examples, and maybe some other peripheral stuff. But this workflow doesn't run examples right now, so bringing in examples-only dependencies is bloat (at the level of what's installed) at the benefit of simplicity (only declare one "package")

So here I'd advocate for either

  1. Swap out openff-toolkit-examples for openff-toolkit and whatever other packages are needed for the tests that currently run (my preference) or
  2. Update this workflow to also run examples nightly from the latest conda package

(There's also the 1a option of merging this as-is to get stuff green for tonight's run, and then following it up with a change later on that brings back 3.11 and installs without openmmforcefields.)

j-wags commented 2 months ago

this workflow doesn't run examples right now

I'm glad I got your eyes on this! That is... a huge oversight on my part. I thought this WAS running examples 🤦

I'll try option 2 - updating this PR so the nightly conda builds do test examples.

j-wags commented 2 months ago

I'm manually kicking off conda tests here