Skip to content

Commit

Permalink
Merge pull request #1104 from ocefpaf/fix_windows_tests
Browse files Browse the repository at this point in the history
Fix windows tests
  • Loading branch information
benjwadams authored Aug 29, 2024
2 parents d657152 + 3213f6d commit b164024
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 51 deletions.
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ repos:
- test_requirements.txt

- repo: https://github.com/psf/black
rev: 24.4.2
rev: 24.8.0
hooks:
- id: black
language_version: python3
Expand All @@ -31,12 +31,12 @@ repos:


- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.0
rev: v0.5.7
hooks:
- id: ruff

- repo: https://github.com/tox-dev/pyproject-fmt
rev: 2.1.3
rev: 2.2.1
hooks:
- id: pyproject-fmt

Expand Down
4 changes: 2 additions & 2 deletions compliance_checker/cf/cf_1_6.py
Original file line number Diff line number Diff line change
Expand Up @@ -3340,7 +3340,7 @@ def check_packed_data(self, ds):

# IMPLEMENTATION CONFORMANCE 8.1 REQUIRED 1/3
# scale_factor and add_offset same type
if type(add_offset) != type(scale_factor):
if not isinstance(add_offset, type(scale_factor)):
valid = False
reasoning.append(
"Attributes add_offset and scale_factor have different data type.",
Expand All @@ -3350,7 +3350,7 @@ def check_packed_data(self, ds):
# if not the same type
# FIXME: Check add_offset too.

elif type(scale_factor) != var.dtype.type:
elif not isinstance(scale_factor, var.dtype.type):
# Check both attributes are type float or double
if not isinstance(scale_factor, (float, np.floating)):
valid = False
Expand Down
25 changes: 18 additions & 7 deletions compliance_checker/cf/cf_1_9.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ def check_domain_variables(self, ds: Dataset):
continue
appendix_a_not_recommended_attrs = []
for attr_name in domain_var.ncattrs():
if attr_name in self.appendix_a and "D" not in self.appendix_a[attr_name]["attr_loc"]:
if (
attr_name in self.appendix_a
and "D" not in self.appendix_a[attr_name]["attr_loc"]
):
appendix_a_not_recommended_attrs.append(attr_name)

if appendix_a_not_recommended_attrs:
Expand All @@ -163,21 +166,29 @@ def check_domain_variables(self, ds: Dataset):
# no errors occurred
domain_valid.score += 1


# IMPLEMENTATION CONFORMANCE 5.8 REQUIRED 4/4
if hasattr(domain_var, "cell_measures"):
cell_measures_var_names = regex.findall(r"\b(?:area|volume):\s+(\w+)", domain_var.cell_measures)
cell_measures_var_names = regex.findall(
r"\b(?:area|volume):\s+(\w+)",
domain_var.cell_measures,
)
# check exist
for var_name in cell_measures_var_names:
try:
cell_measures_variable = ds.variables[var_name]
except ValueError:
# TODO: what to do here?
continue
domain_coord_var_names = {var_like.name for var_like in domain_coord_vars}
domain_valid.assert_true(set(cell_measures_variable.dimensions).issubset(domain_coord_var_names),
"Variables named in the cell_measures attributes must have a dimensions attribute with "
"values that are a subset of the referring domain variable's dimension attribute")
domain_coord_var_names = {
var_like.name for var_like in domain_coord_vars
}
domain_valid.assert_true(
set(cell_measures_variable.dimensions).issubset(
domain_coord_var_names,
),
"Variables named in the cell_measures attributes must have a dimensions attribute with "
"values that are a subset of the referring domain variable's dimension attribute",
)

results.append(domain_valid.to_result())

Expand Down
15 changes: 10 additions & 5 deletions compliance_checker/protocols/zarr.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import platform
import zipfile
from pathlib import Path
from urllib.parse import urlparse
Expand All @@ -6,7 +7,11 @@

from compliance_checker.protocols import netcdf

#

def _fix_windows_slashes(zarr_url):
if platform.system() == "Windows":
zarr_url = zarr_url.replace("///", "//")
return zarr_url


def is_zarr(url):
Expand Down Expand Up @@ -55,10 +60,9 @@ def as_zarr(url):
pr = urlparse(str(url))

if "mode=nczarr" in pr.fragment:
if pr.netloc:
return str(url) # already valid nczarr url
elif pr.scheme == "file":
return str(url) # already valid nczarr url
if pr.netloc or pr.scheme == "file":
url = _fix_windows_slashes(url)
return url

zarr_url = Path(
url2pathname(pr.path),
Expand All @@ -78,4 +82,5 @@ def as_zarr(url):
url_base = url if mode == "s3" else zarr_url.as_uri()

zarr_url = f"{url_base}#mode=nczarr,{mode}"
zarr_url = _fix_windows_slashes(zarr_url)
return zarr_url
5 changes: 0 additions & 5 deletions compliance_checker/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import inspect
import itertools
import os
import platform
import re
import subprocess
import sys
Expand Down Expand Up @@ -893,10 +892,6 @@ def load_local_dataset(self, ds_str):
ds_str = self.generate_dataset(ds_str)

if zarr.is_zarr(ds_str):
if platform.system() != "Linux":
print(
f"WARNING: {platform.system()} OS detected. NCZarr is not officially supported for your OS as of when this API was written. Your mileage may vary.",
)
return Dataset(zarr.as_zarr(ds_str))

if netcdf.is_netcdf(ds_str):
Expand Down
12 changes: 7 additions & 5 deletions compliance_checker/tests/test_cf.py
Original file line number Diff line number Diff line change
Expand Up @@ -3249,7 +3249,7 @@ def test_domain(self):
dataset.createDimension("lat", 20)
dataset.createDimension("depth", 20)
domain_var.setncattr("dimensions", "lon lat depth")
cube = dataset.createVariable("cube", "f8", ("lon", "lat", "depth"))
dataset.createVariable("cube", "f8", ("lon", "lat", "depth"))
# OK, coordinates in cell_measures are subset of coordinates of
# referring domain variable's coordinates attribute
results = self.cf.check_domain_variables(dataset)
Expand All @@ -3258,10 +3258,12 @@ def test_domain(self):
domain_var.cell_measures = "volume: cube_bad"
dataset.createVariable("cube_bad", "f8", ("lon", "lat", "depth", "time"))
results = self.cf.check_domain_variables(dataset)
self.assertTrue("Variables named in the cell_measures attributes must "
"have a dimensions attribute with values that are a "
"subset of the referring domain variable's dimension "
"attribute" in results[0].msgs)
self.assertTrue(
"Variables named in the cell_measures attributes must "
"have a dimensions attribute with values that are a "
"subset of the referring domain variable's dimension "
"attribute" in results[0].msgs,
)
del dataset
dataset = MockTimeSeries()
# domain should be dimensionless -- currently not an error in
Expand Down
47 changes: 23 additions & 24 deletions compliance_checker/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@

from .conftest import datadir, static_files

on_windows = platform.system() == "Windows"

if on_windows:
ncconfig = ["sh", f"{os.environ['CONDA_PREFIX']}\\Library\\bin\\nc-config"]
else:
ncconfig = ["nc-config"]


@pytest.mark.usefixtures("checksuite_setup")
class TestCLI:
Expand Down Expand Up @@ -221,32 +228,19 @@ def test_multi_checker_return_value(self, tmp_txt_file):
assert not return_value

def _check_libnetcdf_version():
if platform.system() == "Linux":
# nc-config doesn't work on windows... and neither does NCZarr so this skipif is mutually exclusive to the OS check skipif
return (
float(
subprocess.check_output(
["nc-config", "--version"],
encoding="UTF-8",
)[9:12],
)
< 8.0
return (
float(
subprocess.check_output(
ncconfig + ["--version"],
encoding="UTF-8",
)[9:12],
)
else:
return True
< 8.0
)

# TODO uncomment the third parameter once S3 support is working
@pytest.mark.skipif(
_check_libnetcdf_version(),
reason="NCZarr support was not available until netCDF version 4.8.0. Please upgrade to the latest libnetcdf version to test this functionality",
)
@pytest.mark.skipif(
platform.system() != "Linux",
reason="NCZarr is not officially supported for your OS as of when this API was written",
)
@pytest.mark.skipif(
subprocess.check_output(["nc-config", "--has-nczarr"]) != b"yes\n",
reason="NCZarr support was not built with this netCDF version",
subprocess.check_output(ncconfig + ["--has-nczarr"]) != b"yes\n",
reason="NCZarr is not available.",
)
@pytest.mark.parametrize(
"zarr_url",
Expand All @@ -255,7 +249,12 @@ def _check_libnetcdf_version():
str(datadir / "zip.zarr"),
# "s3://hrrrzarr/sfc/20210408/20210408_10z_anl.zarr#mode=nczarr,s3"
],
ids=["local_file", "zip_file"], # ,'s3_url'
ids=[
"local_file",
"zip_file",
# TODO uncomment once S3 support is working.
# "s3_url",
],
)
def test_nczarr_pass_through(self, zarr_url):
"""
Expand Down

0 comments on commit b164024

Please sign in to comment.