OpenFreeEnergy / openfe

The Open Free Energy toolkit
https://docs.openfree.energy
MIT License
136 stars 19 forks source link

Tests in test_openmm_equil_rfe_protocols.py fail with pytest=7.4.0 #507

Open hannahbaumann opened 1 year ago

hannahbaumann commented 1 year ago

I created a new conda env from the environment.yml file and got mulitple errors in the test_openmm_equil_rfe_protocols.py, e.g.

file /Users/hannahbaumann/openfe/openfe/tests/protocols/test_openmm_equil_rfe_protocols.py, line 38 def test_append_topology(benzene_complex_system, toluene_complex_system): file /Users/hannahbaumann/openfe/openfe/tests/protocols/conftest.py, line 29 @pytest.fixture def benzene_complex_system(benzene_modifications, T4_protein_component): E fixture 'benzene_modifications' not found

Downgrading pytest to 7.3.1 seems to fix the issue.

dwhswenson commented 1 year ago

CI is working fine with pytest 7.4 (such as this run), and I just tried 7.4 locally and it worked, so that shouldn't be the problem.

  1. What Python version are you using? You may be defaulting to 3.11, which we don't test against; there could be issues with that.
  2. Have you done a developer install in this environment?
  3. How exactly are you invoking pytest? (From the openfe test command? pytest for a specific file? For the whole suite? Using --pyargs?)
hannahbaumann commented 1 year ago

Hm, that's weird! Here what I did:

  1. Yes, I'm using Python 3.11.4
  2. Yes, this is a developer install
  3. I had run them in pycharm (for a specific file). Right now runnign the openfe test --long command and that also raises a lot of errors.
dwhswenson commented 1 year ago

We don't test against 3.11, so I'd recommend downgrading that to 3.10. I don't recall what we're waiting on before we can add 3.11 to the test matrix; maybe the recent ambertools build allows us to now run on 3.11?

IAlibay commented 1 year ago

Yeah I think it was OpenFF TK and then by correspondence ambertools. Should be ok to upgrade now.

hannahbaumann commented 1 year ago

Awesome, thank you for helping figure this out!

IAlibay commented 1 year ago

So interestingly py311 is working fine on CI, but I am also about to replicate this 7.4.0 issue on a clean install.

dwhswenson commented 1 year ago
  1. Is it only with py3.11 and pytest 7.4.0? (i.e., do you reproduce in 3.10/7.4.0)
  2. Is it only for developer installs?
  3. What happens if you take the session scoping off of the benzene_modifications fixture?
IAlibay commented 1 year ago

Only tried on developer installs with 7.4.0 + py3.11, I'll see if I can reproduce later.

My issue was with another fixture (not benzene_modifications, but I didn't note down exactly which one).