Ensure tests use ncgen/ncdump from the Python env; rationalise uses o…

…f ncgen. (#4794)

* Ensure tests use ncgen/ncdump from the Python env; rationalise uses of ncgen a bit.

* Tidy some imports.

* Rationalise use of ncgen and avoid subprocess 'shell=True'.

* Review changes: revert to importing iris.tests first.

* Tidy routines slightly.

* Fix subprocess call.

* Added whatsnew.

docs/src/whatsnew/latest.rst

@@ -216,6 +216,12 @@ This document explains the changes made to Iris for this release
 #. `@bjlittle`_ migrated to GitHub Actions for Continuous-Integration.
  (:pull:`4503`)
 
+#. `@pp-mo`_ made tests run certain linux executables from the Python env,
+ specifically ncdump and ncgen. These could otherwise fail when run in IDEs
+ such as PyCharm and Eclipse, which don't automatically include the Python env
+ bin in the system PATH.
+ (:pull:`4794`)
+
 
 .. comment
  Whatsnew author names (@github name) in alphabetical order. Note that,

lib/iris/tests/__init__.py

@@ -34,12 +34,13 @@
 import math
 import os
 import os.path
+from pathlib import Path
 import re
 import shutil
 import subprocess
 import sys
 import threading
-from typing import Dict, List
+from typing import AnyStr, Dict, List
 import unittest
 from unittest import mock
 import warnings
@@ -373,8 +374,8 @@ def assertCDL(self, netcdf_filename, reference_filename=None, flags="-h"):
  flags = list(map(str, flags))
 
  try:
- # Python3 only: use subprocess.run()
- args = ["ncdump"] + flags + [netcdf_filename]
+ exe_path = env_bin_path("ncdump")
+ args = [exe_path] + flags + [netcdf_filename]
  cdl = subprocess.check_output(args, stderr=subprocess.STDOUT)
  except subprocess.CalledProcessError as exc:
  print(exc.output)
@@ -1348,3 +1349,30 @@ def wrapped(self, *args, **kwargs):
  return result
 
  return wrapped
+
+
+def env_bin_path(exe_name: AnyStr = None):
+ """
+ Return a Path object for (an executable in) the environment bin directory.
+
+ Parameters
+ ----------
+ exe_name : str
+ If set, the name of an executable to append to the path.
+
+ Returns
+ -------
+ exe_path : Path
+ A path to the bin directory, or an executable file within it.
+
+ Notes
+ -----
+ For use in tests which spawn commands which should call executables within
+ the Python environment, since many IDEs (Eclipse, PyCharm) don't
+ automatically include this location in $PATH (as opposed to $PYTHONPATH).
+ """
+ exe_path = Path(os.__file__)
+ exe_path = (exe_path / "../../../bin").resolve()
+ if exe_name is not None:
+ exe_path = exe_path / exe_name
+ return exe_path

lib/iris/tests/integration/experimental/test_ugrid_save.py

@@ -14,17 +14,15 @@
 import glob
 from pathlib import Path
 import shutil
-from subprocess import check_call
 import tempfile
 
 import iris
 from iris.experimental.ugrid.load import PARSE_UGRID_ON_LOAD
 import iris.fileformats.netcdf
-from iris.tests import IrisTest
-from iris.tests.stock.netcdf import _add_standard_data
+from iris.tests.stock.netcdf import _add_standard_data, ncgen_from_cdl
 
 
-class TestBasicSave(IrisTest):
+class TestBasicSave(tests.IrisTest):
  @classmethod
  def setUpClass(cls):
  cls.temp_dir = Path(tempfile.mkdtemp())
@@ -46,11 +44,11 @@ def tearDownClass(cls):
 
  def test_example_result_cdls(self):
  # Snapshot the result of saving the example cases.
- for ex_name, filepath in self.example_names_paths.items():
+ for ex_name, cdl_path in self.example_names_paths.items():
+ # Create a test netcdf file.
  target_ncfile_path = str(self.temp_dir / f"{ex_name}.nc")
- # Create a netcdf file from the test CDL.
- check_call(
- f"ncgen {filepath} -k4 -o {target_ncfile_path}", shell=True
+ ncgen_from_cdl(
+ cdl_str=None, cdl_path=cdl_path, nc_path=target_ncfile_path
  )
  # Fill in blank data-variables.
  _add_standard_data(target_ncfile_path)
@@ -64,18 +62,18 @@ def test_example_result_cdls(self):
  refdir_relpath = (
  "integration/experimental/ugrid_save/TestBasicSave/"
  )
- reffile_name = str(Path(filepath).name).replace(".nc", ".cdl")
+ reffile_name = str(Path(cdl_path).name).replace(".nc", ".cdl")
  reffile_path = refdir_relpath + reffile_name
  self.assertCDL(resave_ncfile_path, reference_filename=reffile_path)
 
  def test_example_roundtrips(self):
  # Check that save-and-loadback leaves Iris data unchanged,
  # for data derived from each UGRID example CDL.
- for ex_name, filepath in self.example_names_paths.items():
+ for ex_name, cdl_path in self.example_names_paths.items():
+ # Create a test netcdf file.
  target_ncfile_path = str(self.temp_dir / f"{ex_name}.nc")
- # Create a netcdf file from the test CDL.
- check_call(
- f"ncgen {filepath} -k4 -o {target_ncfile_path}", shell=True
+ ncgen_from_cdl(
+ cdl_str=None, cdl_path=cdl_path, nc_path=target_ncfile_path
  )
  # Fill in blank data-variables.
  _add_standard_data(target_ncfile_path)

lib/iris/tests/integration/test_climatology.py

@@ -13,14 +13,14 @@
 from os.path import join as path_join
 from os.path import sep as os_sep
 import shutil
-from subprocess import check_call
 import tempfile
 
 import iris
 from iris.tests import stock
+from iris.tests.stock.netcdf import ncgen_from_cdl
 
 
-class TestClimatology(iris.tests.IrisTest):
+class TestClimatology(tests.IrisTest):
  reference_cdl_path = os_sep.join(
  [
  dirname(tests.__file__),
@@ -58,12 +58,13 @@ def setUpClass(cls):
  cls.temp_dir = tempfile.mkdtemp()
  cls.path_ref_cdl = path_join(cls.temp_dir, "standard.cdl")
  cls.path_ref_nc = path_join(cls.temp_dir, "standard.nc")
- # Create reference CDL file.
- with open(cls.path_ref_cdl, "w") as f_out:
- f_out.write(cls._simple_cdl_string())
- # Create reference netCDF file from reference CDL.
- command = "ncgen -o {} {}".format(cls.path_ref_nc, cls.path_ref_cdl)
- check_call(command, shell=True)
+ # Create reference CDL and netcdf files (with ncgen).
+ ncgen_from_cdl(
+ cdl_str=cls._simple_cdl_string(),
+ cdl_path=cls.path_ref_cdl,
+ nc_path=cls.path_ref_nc,
+ )
+
  cls.path_temp_nc = path_join(cls.temp_dir, "tmp.nc")
 
  # Create reference cube.

lib/iris/tests/integration/test_netcdf.py

@@ -14,7 +14,6 @@
 import os.path
 from os.path import join as path_join
 import shutil
-from subprocess import check_call
 import tempfile
 from unittest import mock
 import warnings
@@ -33,6 +32,7 @@
  UnknownCellMethodWarning,
 )
 import iris.tests.stock as stock
+from iris.tests.stock.netcdf import ncgen_from_cdl
 import iris.tests.unit.fileformats.netcdf.test_load_cubes as tlc
 
 
@@ -864,12 +864,12 @@ def setUpClass(cls):
  cls.temp_dir = tempfile.mkdtemp()
  cls.path_test_cdl = path_join(cls.temp_dir, "geos_problem.cdl")
  cls.path_test_nc = path_join(cls.temp_dir, "geos_problem.nc")
- # Create a reference file from the CDL text.
- with open(cls.path_test_cdl, "w") as f_out:
- f_out.write(cls._geostationary_problem_cdl)
- # Call 'ncgen' to make an actual netCDF file from the CDL.
- command = "ncgen -o {} {}".format(cls.path_test_nc, cls.path_test_cdl)
- check_call(command, shell=True)
+ # Create reference CDL and netcdf files from the CDL text.
+ ncgen_from_cdl(
+ cdl_str=cls._geostationary_problem_cdl,
+  cdl_path=cls.path_test_cdl,
+  nc_path=cls.path_test_nc,
+ )
 
  @classmethod
  def tearDownClass(cls):

lib/iris/tests/stock/netcdf.py

@@ -8,12 +8,60 @@
 from pathlib import Path
 from string import Template
 import subprocess
+from typing import Optional
 
 import dask
 from dask import array as da
 import netCDF4
 import numpy as np
 
+from iris.tests import env_bin_path
+
+NCGEN_PATHSTR = str(env_bin_path("ncgen"))
+
+
+def ncgen_from_cdl(
+ cdl_str: Optional[str], cdl_path: Optional[str], nc_path: str
+):
+ """
+ Generate a test netcdf file from cdl.
+
+ Source is CDL in either a string or a file.
+ If given a string, will either save a CDL file, or pass text directly.
+ A netcdf output file is always created, at the given path.
+
+ Parameters
+ ----------
+ cdl_str : str or None
+ String containing a CDL description of a netcdf file.
+ If None, 'cdl_path' must be an existing file.
+ cdl_path : str or None
+ Path of temporary text file where cdl_str is written.
+ If None, 'cdl_str' must be given, and is piped direct to ncgen.
+ nc_path : str
+ Path of temporary netcdf file where converted result is put.
+
+ Notes
+ -----
+ For legacy reasons, the path args are 'str's not 'Path's.
+
+ """
+ if cdl_str and cdl_path:
+ with open(cdl_path, "w") as f_out:
+ f_out.write(cdl_str)
+ if cdl_path:
+ # Create netcdf from stored CDL file.
+ call_args = [NCGEN_PATHSTR, cdl_path, "-k4", "-o", nc_path]
+ call_kwargs = {}
+ else:
+ # No CDL file : pipe 'cdl_str' directly into the ncgen program.
+ if not cdl_str:
+ raise ValueError("Must provide either 'cdl_str' or 'cdl_path'.")
+ call_args = [NCGEN_PATHSTR, "-k4", "-o", nc_path]
+ call_kwargs = dict(input=cdl_str, encoding="ascii")
+
+ subprocess.run(call_args, check=True, **call_kwargs)
+
 
 def _file_from_cdl_template(
  temp_file_dir, dataset_name, dataset_type, template_subs
@@ -39,12 +87,7 @@ def _file_from_cdl_template(
 
  # Spawn an "ncgen" command to create an actual NetCDF file from the
  # CDL string.
- subprocess.run(
- ["ncgen", "-o" + str(nc_write_path)],
- input=cdl,
- encoding="ascii",
- check=True,
- )
+ ncgen_from_cdl(cdl_str=cdl, cdl_path=None, nc_path=nc_write_path)
 
  return nc_write_path
 

lib/iris/tests/test_netcdf.py

@@ -16,7 +16,6 @@
 import os.path
 import shutil
 import stat
-from subprocess import check_call
 import tempfile
 from unittest import mock
 
@@ -33,6 +32,7 @@
 from iris.fileformats.netcdf import load_cubes as nc_load_cubes
 import iris.std_names
 import iris.tests.stock as stock
+from iris.tests.stock.netcdf import ncgen_from_cdl
 import iris.util
 
 
@@ -363,12 +363,8 @@ def test_um_stash_source(self):
  self.tmpdir = tempfile.mkdtemp()
  cdl_path = os.path.join(self.tmpdir, "tst.cdl")
  nc_path = os.path.join(self.tmpdir, "tst.nc")
- # Write CDL string into a temporary CDL file.
- with open(cdl_path, "w") as f_out:
- f_out.write(ref_cdl)
- # Use ncgen to convert this into an actual (temporary) netCDF file.
- command = "ncgen -o {} {}".format(nc_path, cdl_path)
- check_call(command, shell=True)
+ # Create a temporary netcdf file from the CDL string.
+ ncgen_from_cdl(ref_cdl, cdl_path, nc_path)
  # Load with iris.fileformats.netcdf.load_cubes, and check expected content.
  cubes = list(nc_load_cubes(nc_path))
  self.assertEqual(len(cubes), 1)
@@ -412,12 +408,8 @@ def test_ukmo__um_stash_source_priority(self):
  self.tmpdir = tempfile.mkdtemp()
  cdl_path = os.path.join(self.tmpdir, "tst.cdl")
  nc_path = os.path.join(self.tmpdir, "tst.nc")
- # Write CDL string into a temporary CDL file.
- with open(cdl_path, "w") as f_out:
- f_out.write(ref_cdl)
- # Use ncgen to convert this into an actual (temporary) netCDF file.
- command = "ncgen -o {} {}".format(nc_path, cdl_path)
- check_call(command, shell=True)
+ # Create a temporary netcdf file from the CDL string.
+ ncgen_from_cdl(ref_cdl, cdl_path, nc_path)
  # Load with iris.fileformats.netcdf.load_cubes, and check expected content.
  cubes = list(nc_load_cubes(nc_path))
  self.assertEqual(len(cubes), 1)
@@ -457,12 +449,8 @@ def test_bad_um_stash_source(self):
  self.tmpdir = tempfile.mkdtemp()
  cdl_path = os.path.join(self.tmpdir, "tst.cdl")
  nc_path = os.path.join(self.tmpdir, "tst.nc")
- # Write CDL string into a temporary CDL file.
- with open(cdl_path, "w") as f_out:
- f_out.write(ref_cdl)
- # Use ncgen to convert this into an actual (temporary) netCDF file.
- command = "ncgen -o {} {}".format(nc_path, cdl_path)
- check_call(command, shell=True)
+ # Create a temporary netcdf file from the CDL string.
+ ncgen_from_cdl(ref_cdl, cdl_path, nc_path)
  # Load with iris.fileformats.netcdf.load_cubes, and check expected content.
  cubes = list(nc_load_cubes(nc_path))
  self.assertEqual(len(cubes), 1)

lib/iris/tests/unit/experimental/ugrid/load/test_load_meshes.py

@@ -13,7 +13,6 @@
 
 from pathlib import Path
 from shutil import rmtree
-from subprocess import check_call
 import tempfile
 from uuid import uuid4
 
@@ -22,6 +21,7 @@
  load_meshes,
  logger,
 )
+from iris.tests.stock.netcdf import ncgen_from_cdl
 
 
 def setUpModule():
@@ -35,15 +35,11 @@ def tearDownModule():
 
 
 def cdl_to_nc(cdl):
- cdl_path = TMP_DIR / "tst.cdl"
- nc_path = TMP_DIR / f"{uuid4()}.nc"
- # Write CDL string into a temporary CDL file.
- with open(cdl_path, "w") as f_out:
- f_out.write(cdl)
+ cdl_path = str(TMP_DIR / "tst.cdl")
+ nc_path = str(TMP_DIR / f"{uuid4()}.nc")
  # Use ncgen to convert this into an actual (temporary) netCDF file.
- command = "ncgen -o {} {}".format(nc_path, cdl_path)
- check_call(command, shell=True)
- return str(nc_path)
+ ncgen_from_cdl(cdl_str=cdl, cdl_path=cdl_path, nc_path=nc_path)
+ return nc_path
 
 
 class TestsBasic(tests.IrisTest):