roocs / clisops

Climate Simulation Operations
https://clisops.readthedocs.io/en/latest/
Other
21 stars 9 forks source link

Streamline fixtures, use pytest-xdist, drop Python3.8 #345

Closed Zeitsperre closed 3 weeks ago

Zeitsperre commented 2 months ago

Pull Request Checklist:

What kind of change does this PR introduce?:

Does this PR introduce a breaking change?:

Yes. Testing now requires the pytest-xdist plugin (has been added to pyproject.toml and environment.yml). By default, running pytest will only use one worker, but tox will use up to eight workers.

Other information:

https://pytest-xdist.readthedocs.io/en/stable/

cehbrecht commented 2 months ago

@Zeitsperre looks good šŸ‘ ... but probably need some support in adapting PR #342.

Zeitsperre commented 2 months ago

@cehbrecht There's a ton of work to do. I'm completely tearing the testing apart. I might need some help to move mini-esgf-data to a new system that's easier to maintain.

Zeitsperre commented 1 month ago

There are a handful of tests failing that I can't seem to make sense of. It may be due to breaking changes in more recent versions of dask and xarray, but it isn't clear to me which changes are the culprit.

Within some of the internals of clisops (e.g. clisops.ops.base_operation.Operation) there are direct calls to roocs-utils's open_xr_dataset. Is it possible that these helper functions are broken?

One thing for certain is that this affects tests that open paths ending in wildcards (i.e. path/to/file/*.nc). I've tried to load() these on open, but it doesn't seem to have an effect. I'm totally stumped.

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11163828659

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
clisops/utils/testing.py 116 160 72.5%
<!-- Total: 137 181 75.69% -->
Files with Coverage Reduction New Missed Lines %
clisops/ops/base_operation.py 3 87.5%
<!-- Total: 3 -->
Totals Coverage Status
Change from base Build 10473239325: 0.6%
Covered Lines: 1853
Relevant Lines: 2532

šŸ’› - Coveralls
Zeitsperre commented 1 month ago

@cehbrecht Waiting on your review here. Lots of changes, but things are much smoother here.

cehbrecht commented 4 weeks ago

@Zeitsperre thanks for the update. I'm fine with the changes. Moving to pooch looks good. Probably we will need some support for the other PR #342 after the merge of this one. Maybe have a release after the merge of this PR? PR #342 also partially replaces some functions of roocs-utils ... but there is more to do.