Correctness checks for parse_access_ncfile #232
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
runnings tests easier - Added tests to make sure taht we are parsing netCDF files as expected using intake-esm by comparing against a dataset read in directly with xarray (with same additional logic intake-esm uses) - All tests passing when coordinate variable discovery enabled - test_parsed_ncfile_values_0 failing when coordinate discovery disabled
* Added dynamic xfail setting - necessary to ensure that we are correctly determining whether tests should pass or fail based on whether we are using latest intake-esm release or ACCESS-NRI modified version with coord variable detection. Uses a pytest_collection_modifyitems function to create xfails for tests which should fail Merge 226 editable install (#228) * Updated the way that the location of the catalog.yaml file is discovered to work in both editable & regular installations * Changed ci.yml to run tests in a default, rathe than editable installation * Updated pyproject.toml & .github/workflows/ci.yl to ensure correct coverage issues running tests using regular installation rather than editable * Added explicit test for metadata_template - must have previously been implicitly run
…0 to fix integer conversion error if environment variables not specified.
- Relaxed intake version on pyproject.toml to >0.7 - Updated conftest to emit a warning if coordinate discovery is disabled (ie. wrong intake-esm version) and no environment variable is set to inform pytest this is the case
…all in conftest to use keyword arguments rather than positional, updated comments in test_parse_access_ncfile to be more descriptive
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #232 +/- ##
=======================================
Coverage 96.36% 96.36%
=======================================
Files 9 9
Lines 688 688
=======================================
Hits 663 663
Misses 25 25 ☔ View full report in Codecov by Sentry. |
Merged
* Rebased 153 onto 187 * gelaxed intake version on pyproject.toml to >0.7 (take2) & added tox environments for take2
rbeucher
approved these changes
Oct 28, 2024
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #187.
This turned out to be quite a bit more involved than I had hoped for - mostly because we need to toggle what we expect for our test results based on whether we are using the version of intake-esm on either main, or the version which enables coordinate discovery.
I've done so using tox - this is a tool for running tests, linters, etc etc. against a bunch of different Python environments.
I haven't added a tox run to CI because it can be pretty expensive to run (
tox -rtakes approximately 3 and half minutes) on my local machine, but perhaps it's something we should consider?Testing correctness against either permutation of
intake-esm- with or without coords, should be as straightforward as callingtox& the correct environments & dependencies will be triggered.NB: I discovered when setting this up that that
intake-esmsupports python3.10, 3.11 and 3.12. Currently, this package supports 3.9, 3.10 and 3.11. Given we depend on intake-esm, I think we should probably think about updating our supported versions to follow intake-esm.