Correctness checks for parse_access_ncfile (#232)

* - Added some optional test dependencies to pyproject.toml to make
  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

* Remove redundant breakpoint

* Condensed duplicate tests

* * Added tox config to test migration to 2.0.7
* 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

* Changed ox.environ.get('XFAILS', default) from default='' to default=0 to fix integer conversion error if environment variables not specified.

* - Final tox.ini set up, with as sensible factoring as possible
- 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

* Removed some unnecessary dependencies from .[test], updated warning call in conftest to use keyword arguments rather than positional, updated comments in test_parse_access_ncfile to be more descriptive

* Added python3.9 to tox.ini

* Removed python3.9 again (not supported by intake-esm)

* Repinned intake to 0.7.0

* Changes xfails explanation to be more terse

* Made _get_xfail() docstr clearer

* Intake Take2 (#233)

* Rebased 153 onto 187

* gelaxed intake version on pyproject.toml to >0.7 (take2) & added tox environments for take2

pyproject.toml

@@ -16,7 +16,7 @@ classifiers = [
 dependencies = [
     "cftime",
     "ecgtools>=2023.7.13",
-    "intake==0.7.0",
+    "intake>=0.7.0",
     "intake-dataframe-catalog>=0.2.4",
     "intake-esm>=2023.11.10",
     "jsonschema",
@@ -25,6 +25,12 @@ dependencies = [
 ]
 dynamic = ["version"]
 
+[project.optional-dependencies]
+test = [
+    "pytest",
+    "tox",
+] 
+
 [project.scripts]
 catalog-build = "access_nri_intake.cli:build"
 metadata-validate = "access_nri_intake.cli:metadata_validate"

tests/conftest.py

@@ -2,13 +2,57 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import os
+import warnings
 from pathlib import Path
 
 from pytest import fixture
 
 here = os.path.abspath(os.path.dirname(__file__))
 
 
+def _get_xfail():
+    """
+    Get the XFAILS environment variable. We use a default of 1, indicating we expect
+    to add xfail marker to `test_parse_access_ncfile[AccessOm2Builder-access-om2/output000/ocean/ocean_grid.nc-expected0-True]`
+    unless specified.
+    """
+    xfails_default = 1
+
+    try:
+        return int(os.environ["XFAILS"])
+    except KeyError:
+        warnings.warn(
+            message=(
+                "XFAILS enabled by default as coordinate discovery disabled by default. "
+                "This will be deprecated when coordinate discovery is enabled by default"
+            ),
+            category=PendingDeprecationWarning,
+        )
+        return xfails_default
+
+
+_add_xfail = _get_xfail()
+
+
 @fixture(scope="session")
 def test_data():
     return Path(os.path.join(here, "data"))
+
+
+def pytest_collection_modifyitems(config, items):
+    """
+    This function is called by pytest to modify the items collected during test
+    collection. We use it here to mark the xfail tests in
+    test_builders::test_parse_access_ncfile when we check the file contents & to
+    ensure we correctly get xfails if we don't have cordinate discovery enabled
+    in intake-esm.
+    """
+    for item in items:
+        if (
+            item.name
+            in (
+                "test_parse_access_ncfile[AccessOm2Builder-access-om2/output000/ocean/ocean_grid.nc-expected0-True]",
+            )
+            and _add_xfail
+        ):
+            item.add_marker("xfail")

tests/test_builders.py

@@ -6,6 +6,9 @@
 import intake
 import pandas as pd
 import pytest
+import xarray as xr
+from intake_esm.source import _get_xarray_open_kwargs, _open_dataset
+from intake_esm.utils import OPTIONS
 
 from access_nri_intake.source import CORE_COLUMNS, builders
 from access_nri_intake.source.utils import _AccessNCFileInfo
@@ -359,6 +362,13 @@ def test_parse_access_filename(builder, filename, expected):
     assert builder.parse_access_filename(filename) == expected
 
 
+@pytest.mark.parametrize(
+    "compare_files",
+    [
+        (True),
+        (False),
+    ],
+)
 @pytest.mark.parametrize(
     "builder, filename, expected",
     [
@@ -1088,10 +1098,39 @@ def test_parse_access_filename(builder, filename, expected):
         ),
     ],
 )
-def test_parse_access_ncfile(test_data, builder, filename, expected):
+def test_parse_access_ncfile(test_data, builder, filename, expected, compare_files):
     file = str(test_data / Path(filename))
 
     # Set the path to the test data directory
     expected.path = file
 
     assert builder.parse_access_ncfile(file) == expected
+
+    if not compare_files:
+        return None
+
+    """
+    In the rest of this test, we refer to the dataset loaded using intake-esm
+    as ie_ds and the dataset loaded directly with xarray as xr_ds.
+
+    We also need to perform some additional logic that intake-esm does to avoid
+    xr.testing.assert_equal from failing due to preprocessing differences.
+    """
+    xarray_open_kwargs = _get_xarray_open_kwargs("netcdf")
+
+    ie_ds = _open_dataset(
+        urlpath=expected.path,
+        varname=expected.variable,
+        xarray_open_kwargs=xarray_open_kwargs,
+        requested_variables=expected.variable,
+    ).compute()
+    ie_ds.set_coords(set(ie_ds.variables) - set(ie_ds.attrs[OPTIONS["vars_key"]]))
+
+    xr_ds = xr.open_dataset(file, **xarray_open_kwargs)
+
+    scalar_variables = [v for v in xr_ds.data_vars if len(xr_ds[v].dims) == 0]
+    xr_ds = xr_ds.set_coords(scalar_variables)
+
+    xr_ds = xr_ds[expected.variable]
+
+    xr.testing.assert_equal(ie_ds, xr_ds)

tox.ini

@@ -0,0 +1,143 @@
+[tox]
+env_list =
+    py{310,311}-main,
+    py{310,311}-coords,
+    py{310,311}-take2,
+    py{310,311}-take2coords
+
+minversion = 4.23.0
+
+[testenv]
+description = Run the tests with pytest
+package = wheel
+wheel_build_env = .pkg
+deps =
+    pytest
+    .[test]
+commands = 
+    pytest  \
+    -W ignore::UserWarning \
+    ; Unable to parse $N assets
+    ; Frequency derived from filename does not match frequency determined from file contents
+           -W ignore::DeprecationWarning \
+    ; PydanticDeprecatedSince20 Warning
+           -W ignore::RuntimeWarning \
+    ; Numpy ABI mismatch warning
+              {tty:--color=yes} {posargs:tests}
+
+[testenv:coordsdisabled]
+setenv =
+    XFAILS=1
+    ; We expect correctness checks to fail here because coordinate discovery isn't 
+    ; enabled in the main branch of intake-esm. This toggles on xfail marks in
+    ; conftest.py.  
+
+[testenv:coordsenabled]
+setenv =
+    XFAILS=0
+    ; We expect correctness checks to pass here because coordinate discovery is
+    ; enabled in the main branch of intake-esm. This disables xfail marks in conftest.py.
+
+[testenv:base-main]
+description = Pin the intake version to 0.7.0, run pytest
+deps =
+    {[testenv]deps}
+    git+https://github.com/ACCESS-NRI/intake-dataframe-catalog.git@main#egg=intake_dataframe_catalog
+    intake==0.7.0
+
+
+[testenv:base-coords]
+description = Use the ACCESS-NRI fork of intake-esm, branch issue_660-coords
+deps = 
+    {[testenv]deps}
+    git+https://github.com/ACCESS-NRI/intake-esm.git@issue_660-coords#egg=intake-esm
+    git+https://github.com/ACCESS-NRI/intake-dataframe-catalog.git@main#egg=intake_dataframe_catalog
+    intake==0.7.0
+
+[testenv:base-take2]
+description = Pin the intake version to 0.7.0, run pytest
+deps =
+    {[testenv]deps}
+    git+https://github.com/ACCESS-NRI/intake-dataframe-catalog.git@take2#egg=intake_dataframe_catalog
+    intake>=2.0.0
+
+
+[testenv:base-take2coords]
+description = Use the ACCESS-NRI fork of intake-esm, branch issue_660-coords
+deps = 
+    {[testenv]deps}
+    git+https://github.com/ACCESS-NRI/intake-esm.git@take2-coords#egg=intake-esm
+    git+https://github.com/ACCESS-NRI/intake-dataframe-catalog.git@take2#egg=intake_dataframe_catalog
+    intake>=2.0.0
+
+[testenv:py39-main]
+basepython = python3.9
+deps = {[testenv:base-main]deps}
+setenv = {[testenv:coordsdisabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py310-main]
+basepython = python3.10
+deps = {[testenv:base-main]deps}
+setenv = {[testenv:coordsdisabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py311-main]
+basepython = python3.11
+deps = {[testenv:base-main]deps}
+setenv = {[testenv:coordsdisabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py39-coords]
+basepython = python3.9
+deps = {[testenv:base-coords]deps}
+setenv = {[testenv:coordsenabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py310-coords]
+basepython = python3.10
+deps = {[testenv:base-coords]deps}
+setenv = {[testenv:coordsenabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py311-coords]
+basepython = python3.11
+deps = {[testenv:base-coords]deps}
+setenv = {[testenv:coordsenabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py39-take2]
+basepython = python3.9
+deps = {[testenv:base-take2]deps}
+setenv = {[testenv:coordsdisabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py310-take2]
+basepython = python3.10
+deps = {[testenv:base-take2]deps}
+setenv = {[testenv:coordsdisabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py311-take2]
+basepython = python3.11
+deps = {[testenv:base-take2]deps}
+setenv = {[testenv:coordsdisabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py39-take2coords]
+basepython = python3.9
+deps = {[testenv:base-take2coords]deps}
+setenv = {[testenv:coordsenabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py310-take2coords]
+basepython = python3.10
+deps = {[testenv:base-take2coords]deps}
+setenv = {[testenv:coordsenabled]setenv}
+commands = {[testenv]commands}   
+
+[testenv:py311-take2coords]
+basepython = python3.11
+deps = {[testenv:base-take2coords]deps}
+setenv = {[testenv:coordsenabled]setenv}
+commands = {[testenv]commands}