From f5d25be88877ff83b1e02fe3e95e9ac2c4ad3d53 Mon Sep 17 00:00:00 2001 From: Xylar Asay-Davis Date: Sun, 23 Aug 2026 06:42:06 -0500 Subject: [PATCH 1/4] Fix panels dropped from projection comparison plots under matplotlib 3.11 `plot_projection_comparison()` was the only one of the three comparison plotting functions that called `ax.set_title()` without an explicit `y`. Matplotlib only auto-positions a title when `y is None`, and that auto-positioning inspects the surrounding artists -- including the cartopy gridline labels, whose bounding boxes are non-finite while the figure is being drawn. The title therefore ended up at y = inf, which left the whole map axes with a non-finite tight bounding box (x0/x1 NaN, y1 inf). The subsequent `savefig(bbox_inches='tight')` in `shared/plot/save.py` unions only the finite axes -- the colorbars -- so the saved figure began at the first colorbar and the leading map panel was cropped out of the image entirely, with all three subplot titles missing. This is visible with matplotlib 3.11 (it does not reproduce on 3.10), and affects roughly 54% of the images in a zppy `comprehensive_v2` MPAS-Analysis run -- every ocean and sea-ice map that goes through this function. Passing an explicit `y` disables the auto-positioning, matching what `plot_polar_comparison()` (y=1.06) and `plot_global_comparison()` (y=1.02) already did. Reproducer, antarctic_extended sea ice concentration: before: 2620x1195 on matplotlib 3.11.1, 3575x1242 on 3.10.9 after: 3576x1242 on matplotlib 3.11.1, 3575x1242 on 3.10.9 Similar in spirit to E3SM-Project/polaris#635 and MPAS-Dev/compass#972. Co-Authored-By: Claude Opus 5 --- mpas_analysis/shared/plot/climatology_map.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mpas_analysis/shared/plot/climatology_map.py b/mpas_analysis/shared/plot/climatology_map.py index 8be601a78..bb5a8262a 100644 --- a/mpas_analysis/shared/plot/climatology_map.py +++ b/mpas_analysis/shared/plot/climatology_map.py @@ -597,7 +597,13 @@ def _plot_panel(ax, title, array, colormap, norm, levels, ticks, contours, lineWidth, lineColor, arrowSpacing, arrowWidth): title = limit_title(title, maxTitleLength) - ax.set_title(title, **plottitle_font) + # Pass an explicit `y` so matplotlib does not auto-position the + # title. Auto-positioning inspects the cartopy gridline labels, + # whose bounding boxes are non-finite while the figure is being + # drawn; the title then lands at y = inf, the axes bounding box + # becomes non-finite and savefig(bbox_inches='tight') drops the + # panel from the figure entirely. + ax.set_title(title, y=1.06, **plottitle_font) ax.set_extent(extent, crs=projection) From 0e56e23c5d52d44f2efde91d341df97cd523ea65 Mon Sep 17 00:00:00 2001 From: Xylar Asay-Davis Date: Sun, 23 Aug 2026 06:53:20 -0500 Subject: [PATCH 2/4] Replace distutils with shutil in the test fixtures `distutils` was removed from the standard library in Python 3.12, so `from distutils import dir_util` only kept working through the shim that setuptools installs. That made collecting anything under `mpas_analysis/test` fail outright in an environment without setuptools. `shutil.copytree(..., dirs_exist_ok=True)` is the direct replacement for `dir_util.copy_tree` onto an existing directory. Co-Authored-By: Claude Opus 5 --- mpas_analysis/test/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mpas_analysis/test/__init__.py b/mpas_analysis/test/__init__.py index 7788d1aa9..0a1465227 100644 --- a/mpas_analysis/test/__init__.py +++ b/mpas_analysis/test/__init__.py @@ -9,7 +9,7 @@ from contextlib import contextmanager import os -from distutils import dir_util +import shutil from pytest import fixture import xarray @@ -53,7 +53,7 @@ def loaddatadir(request, tmpdir): test_dir, _ = os.path.splitext(filename) if os.path.isdir(test_dir): - dir_util.copy_tree(test_dir, str(tmpdir)) + shutil.copytree(test_dir, str(tmpdir), dirs_exist_ok=True) request.cls.datadir = tmpdir From d4ee9b507db21c4274064b2073fc204fd9da74dc Mon Sep 17 00:00:00 2001 From: Xylar Asay-Davis Date: Sun, 23 Aug 2026 06:53:20 -0500 Subject: [PATCH 3/4] Add a test that the four dependency lists stay in sync Dependencies are declared in four places -- `dev-spec.txt`, `pixi.toml`, `ci/recipe/recipe.yaml` and `pyproject.toml` -- and nothing kept them consistent, so they could drift silently. The new tests check that: * the three conda lists (dev-spec, pixi, rattler-build recipe) declare exactly the same runtime packages with the same version constraints * conda build pins (currently just the MPI flavor of ESMF) match between dev-spec and pixi * `pyproject.toml` agrees with the conda lists on every shared dependency, and the set it legitimately omits -- packages with no PyPI equivalent -- is exactly the documented `CONDA_ONLY` set, so adding a new conda-only dependency has to be a conscious choice * `requires-python` matches the `python` constraint in all four files * the dev and docs groups agree, allowing `pyproject.toml` to declare build-time requirements under `[build-system] requires` instead Differences in spelling are normalized: conda's `matplotlib-base` against PyPI's `matplotlib`, `_` against `-`, and pixi's `*` against an omitted constraint. The tests skip when run from an installed package, where the dependency files are not present. Verified that perturbing each of the four files is caught by the expected test. Co-Authored-By: Claude Opus 5 --- mpas_analysis/test/test_dependencies.py | 260 ++++++++++++++++++++++++ 1 file changed, 260 insertions(+) create mode 100644 mpas_analysis/test/test_dependencies.py diff --git a/mpas_analysis/test/test_dependencies.py b/mpas_analysis/test/test_dependencies.py new file mode 100644 index 000000000..15aeac8f0 --- /dev/null +++ b/mpas_analysis/test/test_dependencies.py @@ -0,0 +1,260 @@ +# This software is open source software available under the BSD-3 license. +# +# Copyright (c) 2022 Triad National Security, LLC. All rights reserved. +# Copyright (c) 2022 Lawrence Livermore National Security, LLC. All rights +# reserved. +# Copyright (c) 2022 UT-Battelle, LLC. All rights reserved. +# +# Additional copyright and license information can be found in the LICENSE file +# distributed with this code, or at +# https://raw.githubusercontent.com/MPAS-Dev/MPAS-Analysis/main/LICENSE +""" +Check that the dependencies declared in the four places we list them stay in +sync: + +* ``dev-spec.txt`` -- the conda development spec +* ``pixi.toml`` -- the pixi workspace +* ``ci/recipe/recipe.yaml`` -- the rattler-build recipe +* ``pyproject.toml`` -- the PyPI package metadata + +``pyproject.toml`` is deliberately a subset: several dependencies are only +available from conda-forge and have no PyPI equivalent. Those are listed in +``CONDA_ONLY`` below, and the tests check that list is exactly right so that a +new conda-only dependency has to be added consciously. +""" + +import re +import tomllib +from pathlib import Path + +import pytest + +# Packages that appear in the conda dependency lists but deliberately not in +# `pyproject.toml`, because they are not distributed on PyPI (or `python` +# itself, which `pyproject.toml` expresses as `requires-python`). +CONDA_ONLY = { + 'cartopy-offlinedata', + 'esmf', + 'geometric-features', + 'mache', + 'mpas-tools', + 'nco', + 'pyremap', + 'python', +} + +# conda calls it `matplotlib-base`, PyPI calls it `matplotlib` +NAME_ALIASES = {'matplotlib-base': 'matplotlib'} + + +def _repo_root(): + root = Path(__file__).resolve().parents[2] + if not (root / 'pixi.toml').exists(): + pytest.skip('dependency files are not available from an installed ' + 'package; these tests only run from a source checkout') + return root + + +def _canonical(name): + """Normalize a package name for comparison across the four files.""" + name = name.strip().lower().replace('_', '-') + return NAME_ALIASES.get(name, name) + + +def _norm_constraint(constraint): + """Normalize a version constraint. pixi spells "any version" as ``*`` + while the other three files simply leave the constraint off.""" + constraint = constraint.replace(' ', '') + return '' if constraint == '*' else constraint + + +def _split_spec(spec): + """Split e.g. ``cartopy >=0.18.0`` into ``('cartopy', '>=0.18.0')``.""" + spec = spec.strip() + match = re.match(r'^([A-Za-z0-9._-]+)\s*(.*)$', spec) + name, constraint = match.group(1), match.group(2) + return _canonical(name), _norm_constraint(constraint) + + +def _parse_dev_spec(root): + """Parse ``dev-spec.txt`` into ``{section: {name: constraint}}`` plus the + conda build pins (lines of the form ``name=version=build``).""" + sections = {} + builds = {} + current = None + for line in (root / 'dev-spec.txt').read_text().splitlines(): + line = line.strip() + if not line: + continue + if line.startswith('#'): + heading = line.lstrip('#').strip().lower() + if heading in ('base', 'development', 'documentation'): + current = heading + sections[current] = {} + continue + if current is None: + continue + # a conda build pin, e.g. `esmf=*=mpi_mpich_*` + if line.count('=') >= 2 and ' ' not in line: + name, _, build = line.split('=', 2) + builds[_canonical(name)] = build + continue + name, constraint = _split_spec(line) + sections[current][name] = constraint + return sections, builds + + +def _parse_pixi(root): + """Parse ``pixi.toml`` runtime/dev/docs dependencies and build pins.""" + data = tomllib.loads((root / 'pixi.toml').read_text()) + + def convert(table): + deps = {} + builds = {} + for name, value in table.items(): + key = _canonical(name) + if isinstance(value, dict): + deps[key] = _norm_constraint(value.get('version', '*')) + if 'build' in value: + builds[key] = value['build'] + else: + deps[key] = _norm_constraint(value) + return deps, builds + + runtime, builds = convert(data['dependencies']) + features = data.get('feature', {}) + dev, _ = convert(features.get('dev', {}).get('dependencies', {})) + docs, _ = convert(features.get('docs', {}).get('dependencies', {})) + return {'base': runtime, 'development': dev, 'documentation': docs}, builds + + +def _parse_recipe(root): + """Parse the ``requirements: run:`` list out of the rattler-build recipe, + expanding ``${{ ... }}`` references against the recipe ``context``.""" + text = (root / 'ci' / 'recipe' / 'recipe.yaml').read_text() + + context = {} + match = re.search(r'^context:\n((?:[ \t]+\S.*\n)+)', text, re.MULTILINE) + assert match is not None, 'could not find the `context` block in recipe.yaml' + for line in match.group(1).splitlines(): + key, _, value = line.strip().partition(':') + context[key.strip()] = value.strip().strip('"\'') + + match = re.search( + r'^requirements:\n(?:.*\n)*?^ run:\n((?:^ - .*\n)+)', + text, re.MULTILINE) + assert match is not None, \ + 'could not find the `requirements: run:` block in recipe.yaml' + + def expand(value): + return re.sub(r'\$\{\{\s*(\w+)\s*\}\}', + lambda m: context[m.group(1)], value) + + deps = {} + for line in match.group(1).splitlines(): + name, constraint = _split_spec(expand(line.strip()[2:])) + deps[name] = constraint + return deps + + +def _parse_pyproject(root): + data = tomllib.loads((root / 'pyproject.toml').read_text()) + project = data['project'] + runtime = dict(_split_spec(dep) for dep in project['dependencies']) + optional = { + key: dict(_split_spec(dep) for dep in value) + for key, value in project.get('optional-dependencies', {}).items() + } + build = dict(_split_spec(dep) + for dep in data['build-system']['requires']) + return runtime, optional, build, project['requires-python'].replace(' ', '') + + +# Tests ####################################################################### + +def test_conda_runtime_deps_match(): + """dev-spec.txt, pixi.toml and recipe.yaml must list the same runtime + dependencies with the same version constraints.""" + root = _repo_root() + dev_spec, _ = _parse_dev_spec(root) + pixi, _ = _parse_pixi(root) + recipe = _parse_recipe(root) + + assert dev_spec['base'] == pixi['base'], \ + 'dev-spec.txt and pixi.toml runtime dependencies differ' + assert dev_spec['base'] == recipe, \ + 'dev-spec.txt and ci/recipe/recipe.yaml runtime dependencies differ' + + +def test_conda_build_pins_match(): + """Build strings (e.g. the MPI flavor of ESMF) must agree between + dev-spec.txt and pixi.toml.""" + root = _repo_root() + _, dev_spec_builds = _parse_dev_spec(root) + _, pixi_builds = _parse_pixi(root) + assert dev_spec_builds == pixi_builds, \ + 'conda build pins differ between dev-spec.txt and pixi.toml' + + +def test_pyproject_runtime_deps_match_conda(): + """Every dependency shared with the conda lists must carry the same + version constraint, and the conda-only set must be exactly CONDA_ONLY.""" + root = _repo_root() + dev_spec, _ = _parse_dev_spec(root) + conda = dev_spec['base'] + pyproject, _, _, _ = _parse_pyproject(root) + + assert set(pyproject) - set(conda) == set(), \ + 'pyproject.toml lists dependencies missing from the conda lists' + assert set(conda) - set(pyproject) == CONDA_ONLY, \ + ('the set of conda-only dependencies changed; update CONDA_ONLY in ' + 'this test if that is intended') + + mismatched = {name: (constraint, conda[name]) + for name, constraint in pyproject.items() + if constraint != conda[name]} + assert not mismatched, \ + f'version constraints differ (pyproject, conda): {mismatched}' + + +def test_requires_python_consistent(): + """`requires-python` must match the `python` constraint everywhere.""" + root = _repo_root() + dev_spec, _ = _parse_dev_spec(root) + pixi, _ = _parse_pixi(root) + recipe = _parse_recipe(root) + _, _, _, requires_python = _parse_pyproject(root) + + assert dev_spec['base']['python'] == requires_python + assert pixi['base']['python'] == requires_python + assert recipe['python'] == requires_python + + +@pytest.mark.parametrize('section,extra', [('development', 'dev'), + ('documentation', 'docs')]) +def test_extra_deps_match(section, extra): + """The dev and docs dependency groups must agree across dev-spec.txt, + pixi.toml and pyproject.toml. + + `pyproject.toml` may omit build-time requirements that it declares under + `[build-system] requires` instead, so those are allowed to be absent. + """ + root = _repo_root() + dev_spec, _ = _parse_dev_spec(root) + pixi, _ = _parse_pixi(root) + pyproject_runtime, optional, build_requires, _ = _parse_pyproject(root) + + assert dev_spec[section] == pixi[section], \ + f'{section} dependencies differ between dev-spec.txt and pixi.toml' + + declared = optional.get(extra, {}) + missing = set(dev_spec[section]) - set(declared) + assert missing <= set(build_requires), \ + (f'{extra} dependencies missing from pyproject.toml and not declared ' + f'under [build-system] requires: {sorted(missing - set(build_requires))}') + + mismatched = {name: (constraint, dev_spec[section][name]) + for name, constraint in declared.items() + if constraint != dev_spec[section].get(name)} + assert not mismatched, \ + f'{extra} version constraints differ (pyproject, dev-spec): {mismatched}' From d434724d128e2a8df77d6a9337b8a2be36181b97 Mon Sep 17 00:00:00 2001 From: Xylar Asay-Davis Date: Sun, 23 Aug 2026 07:00:26 -0500 Subject: [PATCH 4/4] Stop the test suite writing files into the repository Running `pytest mpas_analysis/test` left four files behind in whatever directory pytest was started from, normally the root of the repo: inset_point.png inset_region.png inset_transect.png PET0.RegridWeightGen.Log The three PNGs came from `test_inset.py`, which passed bare relative filenames to `plt.savefig()`. The ESMF log is written by the `ESMF_RegridWeightGen` subprocess that pyremap launches from `test_climatology.py`; pyremap does not pass a `cwd`, so it lands in the current working directory and cannot be redirected from here. Add an autouse fixture that runs each test in its own temporary working directory. That covers the subprocess case, which is otherwise out of our control, and keeps future tests from reintroducing the problem. All tests find their inputs through absolute paths, so this is safe. Also have `test_inset.py` write to `tmp_path` explicitly rather than relying on the working directory, close its figures, and actually assert that the plot was written -- previously these three tests could not fail. The three near-identical tests become one parametrized test with the same three cases. Co-Authored-By: Claude Opus 5 --- mpas_analysis/test/conftest.py | 30 +++++++++++++ mpas_analysis/test/test_inset.py | 77 +++++++++++--------------------- 2 files changed, 55 insertions(+), 52 deletions(-) create mode 100644 mpas_analysis/test/conftest.py diff --git a/mpas_analysis/test/conftest.py b/mpas_analysis/test/conftest.py new file mode 100644 index 000000000..605db9285 --- /dev/null +++ b/mpas_analysis/test/conftest.py @@ -0,0 +1,30 @@ +# This software is open source software available under the BSD-3 license. +# +# Copyright (c) 2022 Triad National Security, LLC. All rights reserved. +# Copyright (c) 2022 Lawrence Livermore National Security, LLC. All rights +# reserved. +# Copyright (c) 2022 UT-Battelle, LLC. All rights reserved. +# +# Additional copyright and license information can be found in the LICENSE file +# distributed with this code, or at +# https://raw.githubusercontent.com/MPAS-Dev/MPAS-Analysis/main/LICENSE + +import pytest + + +@pytest.fixture(autouse=True) +def run_in_tmp_dir(tmp_path, monkeypatch): + """ + Run every test in its own temporary working directory. + + Some tests -- and the ``ESMF_RegridWeightGen`` subprocess that pyremap + launches, which writes ``PET*.RegridWeightGen.Log`` -- create files + relative to the current working directory. Without this fixture, running + the test suite leaves those files wherever pytest happened to be started, + which is usually the root of the repository. + + All tests locate their inputs through absolute paths (``datadir`` and + ``test_dir`` are both temporary directories), so changing the working + directory is safe. + """ + monkeypatch.chdir(tmp_path) diff --git a/mpas_analysis/test/test_inset.py b/mpas_analysis/test/test_inset.py index 1ab65997b..10db1941d 100644 --- a/mpas_analysis/test/test_inset.py +++ b/mpas_analysis/test/test_inset.py @@ -11,62 +11,35 @@ import matplotlib matplotlib.use('Agg', force=True) -import matplotlib.pyplot as plt -import cartopy.crs as ccrs - -from geometric_features import GeometricFeatures -from mpas_analysis.test import TestCase -from mpas_analysis.shared.plot.inset import add_inset - - -class TestPaths(TestCase): - def test_add_inset_region(self): - # Set up the figure and axes - fig, ax = plt.subplots(figsize=(10, 5), - subplot_kw=dict(projection=ccrs.PlateCarree())) - - # Add coastlines to the map - ax.coastlines() - - gf = GeometricFeatures() - fc = gf.read(componentName='ocean', objectType='region', - featureNames=['North Atlantic Ocean']) - - add_inset(fig, fc) - - # Show the plot - plt.savefig('inset_region.png') - - def test_add_inset_transect(self): - # Set up the figure and axes - fig, ax = plt.subplots(figsize=(10, 5), - subplot_kw=dict(projection=ccrs.PlateCarree())) - - # Add coastlines to the map - ax.coastlines() - - gf = GeometricFeatures() - fc = gf.read(componentName='ocean', objectType='transect', - featureNames=['Drake Passage']) - - add_inset(fig, fc) - - # Show the plot - plt.savefig('inset_transect.png') - - def test_add_inset_point(self): - # Set up the figure and axes - fig, ax = plt.subplots(figsize=(10, 5), - subplot_kw=dict(projection=ccrs.PlateCarree())) - +import matplotlib.pyplot as plt # noqa: E402 +import cartopy.crs as ccrs # noqa: E402 +import pytest # noqa: E402 + +from geometric_features import GeometricFeatures # noqa: E402 +from mpas_analysis.shared.plot.inset import add_inset # noqa: E402 + + +@pytest.mark.parametrize( + 'object_type,feature_name,filename', + [('region', 'North Atlantic Ocean', 'inset_region.png'), + ('transect', 'Drake Passage', 'inset_transect.png'), + ('point', 'Equatorial_Pacific_W155.0_N10.0', 'inset_point.png')]) +def test_add_inset(tmp_path, object_type, feature_name, filename): + # Set up the figure and axes + fig, ax = plt.subplots(figsize=(10, 5), + subplot_kw=dict(projection=ccrs.PlateCarree())) + try: # Add coastlines to the map ax.coastlines() gf = GeometricFeatures() - fc = gf.read(componentName='ocean', objectType='point', - featureNames=['Equatorial_Pacific_W155.0_N10.0']) + fc = gf.read(componentName='ocean', objectType=object_type, + featureNames=[feature_name]) add_inset(fig, fc) - # Show the plot - plt.savefig('inset_point.png') + out_filename = tmp_path / filename + fig.savefig(out_filename) + assert out_filename.exists() + finally: + plt.close(fig)