move test data to zenodo

[atravitz]

resolves #1132

also resolves #1130

Checklist

• switch from using local data to fetching from zenodo
• support running tests offline if data already exists in the cache
• match existing openfe fetch rbfe-tutorial-results behavior: this will require a bit more work on the cli fetching side, I'll address in a follow-up PR

Developers certificate of origin

• I certify that this contribution is covered by the MIT License here and the Developer Certificate of Origin at https://developercertificate.org/.

[mikemhenry]

Lets add an import of from .conftest import HAS_INTERNET and then a mark @pytest.mark.skipif(not HAS_INTERNET, reason="Internet seems to be unavailable") So that we don't get a test fail if someone runs this on an air gapped network

[mikemhenry]

I think b/c of the caching pooch does, we don't need to worry about the fixture scope so these only run once.

Also I know you are still working on this PR, but I did a quick check of all the files we have checked in and once we get the stuff in openfecli/tests/data/ we should be good, some of the other data files are smaller than pdbs and pngs we have in our docs so we don't need to worry about them IMHO (but if they get put onto zenodo then that is fine too)

46M	./openfecli/tests/data/rbfe_results.tar.gz
7.0M	./openfecli/tests/data/rbfe_results_parallel.tar.gz
1004K	./docs/tutorials/assets/t4lyso.png
836K	./openfe/tests/data/cdk8/cdk8_protein.pdb
760K	./openfe/tests/data/openmm_rfe/vacuum_nocoord.nc
664K	./docs/tutorials/tyk2.png
452K	./openfe/tests/data/eg5/eg5_protein.pdb
396K	./openfecli/tests/data/rbfe_tutorial/tyk2_protein.pdb
396K	./docs/tutorials/inputs/tyk2_protein.pdb
336K	./openfe/tests/data/openmm_rfe/RHFEProtocol_json_results.gz
228K	./docs/guide/setup/img/setup_2x.png
216K	./openfe/tests/data/cdk8.zip
208K	./openfe/tests/data/181l_only.pdb
208K	./docs/tutorials/inputs/181L_mod_capped_protonated.pdb
208K	./docs/tutorials/assets/t4_lysozyme.pdb
148K	./docs/guide/protocols/img/protein_2D_RMSD.png
140K	./docs/guide/setup/img/setup_1x.png
132K	./docs/guide/protocols/img/rbfe_thermocycle.png
124K	./docs/guide/protocols/img/ligand_RMSD.png
120K	./docs/guide/protocols/img/ligand_COM_drift.png

[atravitz]

my motivation for putting pooch in a fixture is that it won't get called for tests that don't require it. e.g. if you want to just run the test_format_estimate_uncertainty test, you dont want pooch downloading a test dataset.

[atravitz]

Lets add an import of from .conftest import HAS_INTERNET and then a mark @pytest.mark.skipif(not HAS_INTERNET, reason="Internet seems to be unavailable") So that we don't get a test fail if someone runs this on an air gapped network

I'd like to support these tests in instances where the data exists in the cache, but HAS_INTERNET=False? This would cover most of us doing active development, so that we can 1) do offline development if we already have the data cached 2) test on air-gapped networks by manually uploading data into the cache.

I can brute-force this, but I'm curious if you have an elegant way of going about this?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.88%. Comparing base (064f33d) to head (914a931).
Report is 86 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1148      +/-   ##
==========================================
- Coverage   93.52%   91.88%   -1.64%     
==========================================
  Files         142      142              
  Lines       10632    10628       -4     
==========================================
- Hits         9944     9766     -178     
- Misses        688      862     +174     

Flag	Coverage Δ
fast-tests	91.88% <100.00%> (?)
slow-tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mikemhenry]

Let me think about it -- my first thought is that we check if HAS_INTERNET or OFE_USE_DATA_CACHE (which we make a new envar for) is true, since if either of those are true then we can run the tests that need pooch, but the tests that check if we can fetch tutorial data don't use pooch so we would want to keep the way HAS_INTERNET behaves for them.

Can you test this?

1. Run a test that uses pooch
2. Turn off your internet
3. Run test again

And see if that actually works?

[atravitz]

Let me think about it -- my first thought is that we check if HAS_INTERNET or OFE_USE_DATA_CACHE (which we make a new envar for) is true, since if either of those are true then we can run the tests that need pooch, but the tests that check if we can fetch tutorial data don't use pooch so we would want to keep the way HAS_INTERNET behaves for them.

Can you test this?

1. Run a test that uses pooch

2. Turn off your internet

3. Run test again

And see if that actually works?

Yes that works as expected!

[github-actions]

No API break detected ✅

[atravitz]

see here: https://github.com/OpenFreeEnergy/openfe/pull/1121/files#r1964427513

same data, just correctly(?) compressed.

[mikemhenry]

LGTM -- I don't think we need a news entry since this is pretty back end stuff and functionally it should all work. The only thing I can think of is if we don't add it to the changelog, somewhere we need to document how to manually load the test data on an air gapped system.

[atravitz]

LGTM -- I don't think we need a news entry since this is pretty back end stuff and functionally it should all work. The only thing I can think of is if we don't add it to the changelog, somewhere we need to document how to manually load the test data on an air gapped system.

#1156 working on it here