Cleanup code and fix bugs discovered by additional rules to 'ruff'

.claude/skills/add-reader/SKILL.md

@@ -170,7 +170,7 @@ section.
 `:geo2grid:` prefix inside the `toctree-filt` block.
 
 If the reader serves **both** projects, either drop the prefix (unprefixed entries are always
-included, see `doc/source/toctree_filter.py`) or list it once per prefix — the repo does the latter
+included, see `doc/source/_ext/toctree_filter.py`) or list it once per prefix — the repo does the latter
 for `geotiff` in `doc/source/writers/index.rst`. Either way skip step 7 entirely. Note
 that setting both `is_polar2grid_reader` and `is_geo2grid_reader` does not by itself make a reader
 dual-project in the docs — the toctree entry does.

.claude/skills/build-docs/SKILL.md

@@ -50,12 +50,12 @@ Three mechanisms select content:
    name or script name in a shared page. Available: `|project|`, `|script|` (`polar2grid.sh` /
    `geo2grid.sh`), `|script_literal|`, `|project_env|` (`$POLAR2GRID_HOME` / `$GEO2GRID_HOME`),
    `|cspp_abbr|`, `|cspp_title|`, plus `|ssec|`, `|cspp|`, `|viirs|`.
-2. **`toctree-filt`** (`doc/source/toctree_filter.py`). Prefix entries with `:polar2grid:` or
+2. **`toctree-filt`** (`doc/source/_ext/toctree_filter.py`). Prefix entries with `:polar2grid:` or
    `:geo2grid:`; `toc_filter_exclude` (`conf.py:252`) drops the other project's. It also accepts
    `:excludebuilder: latex`. A page shared by both projects gets no prefix, or is listed once per
    prefix.
 3. **`exclude_patterns`** (`conf.py:266`). A hand-maintained per-project list of files dropped
-   from the build entirely, held in the `_GEO2GRID_EXCLUDES` and `_POLAR2GRID_EXCLUDES` constants.
+   from the build entirely, held in the `geo2grid_excludes` and `polar2grid_excludes` values.
    Inline conditionals use `.. ifconfig:: is_geo2grid`.
 
 ## Adding a page
@@ -74,20 +74,23 @@ WARNING: document isn't included in any toctree
 That means the file is present in a build where nothing references it — add it to that project's
 `exclude_patterns`.
 
-The reverse mistake is caught eagerly: a check at the bottom of `conf.py` validates **both**
-projects' lists and raises `RuntimeError` before the build starts if either names a file that does
-not exist. So deleting or renaming a page means updating the list in the same commit.
+The reverse mistake is caught eagerly: `check_excluded_pages_exist()` in
+`doc/source/_ext/config_checks.py` validates **both** projects' lists on `config-inited` and
+raises `ConfigError` before the build starts if either names a file that does not exist. So
+deleting or renaming a page means updating the list in the same commit.
 
 ## Generated versus hand-maintained
 
 Generated at build time, gitignored, **do not edit**:
 
-- `doc/source/grids_list.rst` — built from `polar2grid/grids/grids.yaml` by `conf.py`. A new
-  built-in grid also needs an entry in the `grid_titles` dict in `conf.py`, or it is silently
-  omitted.
+- `doc/source/grids_list.rst` — built from `polar2grid/grids/grids.yaml` by
+  `doc/source/_ext/grids_list.py`. A new built-in grid also needs an entry in that module's
+  `GRID_TITLES` dict, or it is silently omitted.
 - `doc/source/dev_guide/api/` — `sphinx.ext.apidoc` output (`apidoc_modules` in `conf.py`).
-- `doc/source/_static/example_images/` — ~90 images downloaded from `bin.ssec.wisc.edu` at build
-  time. A new documentation image means adding a URL to the tuple near the top of `conf.py`.
+- `doc/source/_static/example_images/` — images downloaded from `bin.ssec.wisc.edu` at build time
+  by `doc/source/_ext/example_images.py`. A new documentation image means adding a URL to
+  `_POLAR2GRID_IMAGES` or `_GEO2GRID_IMAGES` near the top of `conf.py`; `check_example_images_used()`
+  in `_ext/config_checks.py` fails the build if a listed image is unused or a used one is unlisted.
 
 Generated from Python at build time (edit the Python, not the `.rst`): reader and writer pages use
 `.. automodule::` for the module docstring and the `sphinxarg.ext` `.. argparse::` directive for
@@ -114,6 +117,6 @@ diverged by several releases and overwrote `summary_table.rst` in place). Edit t
 - `conf.py`'s `version` / `release` are the **bundle** versions (Polar2Grid `3.2`, Geo2Grid `1.3`),
   tracking `NEWS.rst` / `NEWS_GEO2GRID.rst`. They are meant to differ from the `polar2grid` package
   version in `pyproject.toml` — do not "sync" them.
-- `doc/source/doi_role.py` provides a `:doi:` role.
+- `doc/source/_ext/doi_role.py` provides a `:doi:` role.
 - PDF output is built with `make latexpdf`; `latex_documents`, `latex_logo`, and
   `latex_appendices` are also project-conditional in `conf.py`.

.github/workflows/ci.yaml

@@ -13,8 +13,12 @@ env:
 jobs:
   # build website
   website:
-    name: Build Documentation
+    name: Build Documentation (${{ matrix.p2g_doc }})
     runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        p2g_doc: ["polar", "geo"]
     steps:
       - name: Checkout source
         uses: actions/checkout@v7
@@ -53,19 +57,12 @@ jobs:
         run: |
           pip install --no-deps -e .
 
-      - name: Build Polar2Grid Website
-        shell: bash -l {0}
-        run: |
-          cd doc
-          make clean
-          make html SPHINXOPTS="-W --keep-going"
-
-      - name: Build Geo2Grid Website
+      - name: Build Website
         shell: bash -l {0}
         run: |
           cd doc
           make clean
-          make html POLAR2GRID_DOC="geo" SPHINXOPTS="-W --keep-going"
+          make html POLAR2GRID_DOC="${{ matrix.p2g_doc }}" SPHINXOPTS="-W --keep-going"
 
   test:
     name: Tests

AGENTS.md

@@ -366,12 +366,18 @@ One source tree builds both projects (`make html` vs `make html POLAR2GRID_DOC=g
 * Use the `rst_epilog` substitutions from `doc/source/conf.py` — `|project|`, `|script|`,
   `|script_literal|`, `|project_env|`, `|cspp_abbr|`, `|cspp_title|`.
 * Use the `toctree-filt` directive with `:polar2grid:` / `:geo2grid:` entry prefixes
-  (`doc/source/toctree_filter.py`), and `.. ifconfig:: is_geo2grid` for inline conditionals.
+  (`doc/source/_ext/toctree_filter.py`), and `.. ifconfig:: is_geo2grid` for inline conditionals.
 * A new `.rst` must be added to a toctree **and** to the *other* project's `exclude_patterns` in
   `conf.py`, or the CI `-W` build fails with "document isn't included in any toctree". The two
-  project lists are the `_GEO2GRID_EXCLUDES` / `_POLAR2GRID_EXCLUDES` constants; a check at the
-  bottom of `conf.py` raises if either one names a file that does not exist, so a page that is
-  removed must also be removed from the list.
+  project lists are the `geo2grid_excludes` / `polar2grid_excludes` configuration values; a
+  `config-inited` check in `doc/source/_ext/config_checks.py` raises if either one names a file
+  that does not exist, so a page that is removed must also be removed from the list.
+
+`doc/source/_ext/` is a package of local Sphinx extensions loaded by the `extensions` list in
+`conf.py`. Alongside `doi_role` and `toctree_filter` it holds the code that used to sit inline in
+`conf.py`: `example_images` (downloads the example images), `grids_list` (generates
+`grids_list.rst`), and `config_checks` (the two `config-inited` consistency checks). Keep
+`conf.py` to configuration and data; put logic here.
 
 Do not hand-edit `doc/source/grids_list.rst`, `doc/source/dev_guide/api/`, or
 `doc/source/_static/example_images/` — all three are generated at build time and gitignored.

doc/source/_ext/__init__.py

@@ -0,0 +1 @@
+"""Local Sphinx extensions for the Polar2Grid and Geo2Grid documentation builds."""

doc/source/_ext/config_checks.py

@@ -0,0 +1,75 @@
+"""Consistency checks for the documentation configuration.
+
+One source tree builds both the Polar2Grid and the Geo2Grid site, and which pages belong
+to which project is expressed by hand in ``conf.py``. These checks run on ``config-inited``
+and fail the build when those hand-maintained lists drift away from the files on disk.
+
+Sphinx already errors on a page that references a missing image, so the half of
+:func:`check_example_images_used` that earns its keep is the other direction: entries that
+no built page uses. The example image list carried 29 such leftovers before it was split
+per project.
+"""
+
+import fnmatch
+import glob
+import os
+import re
+from pathlib import Path
+
+from sphinx.errors import ConfigError
+
+#: Captures the file name from a ``_static/example_images/<name>`` reference.
+_EXAMPLE_IMAGE_RE = re.compile(r"_static/example_images/([A-Za-z0-9_.\-]+)")
+
+#: Characters that make an ``exclude_patterns`` entry a glob rather than a literal path.
+_GLOB_CHARS = "*?["
+
+
+def check_excluded_pages_exist(app, config):
+    """Fail if a literal ``exclude_patterns`` entry names a file that does not exist.
+
+    Both projects' lists are checked, not just the one being built, so a page that is
+    renamed or removed is caught by whichever site builds first.
+    """
+    srcdir = Path(app.srcdir)
+    all_excludes = set(config.exclude_patterns) | set(config.geo2grid_excludes) | set(config.polar2grid_excludes)
+    missing = sorted(
+        pattern
+        for pattern in all_excludes
+        if not any(glob_char in pattern for glob_char in _GLOB_CHARS) and not (srcdir / pattern).exists()
+    )
+    if missing:
+        raise ConfigError("'exclude_patterns' names files that do not exist: " + ", ".join(missing))
+
+
+def check_example_images_used(app, config):
+    """Fail if this project's example image list and the pages being built disagree."""
+    referenced = _referenced_example_images(Path(app.srcdir), config.exclude_patterns)
+    listed = {os.path.basename(image_url) for image_url in config.example_images}
+    if unused := sorted(listed - referenced):
+        raise ConfigError("example image list has entries no built page uses: " + ", ".join(unused))
+    if unlisted := sorted(referenced - listed):
+        raise ConfigError("example image list is missing images used by pages: " + ", ".join(unlisted))
+
+
+def _referenced_example_images(srcdir, exclude_patterns):
+    """Collect the example image file names referenced by the pages this build includes."""
+    referenced = set()
+    for rel_path in glob.glob("**/*.rst", root_dir=srcdir, recursive=True):
+        if any(fnmatch.fnmatch(rel_path, pattern) for pattern in exclude_patterns):
+            continue
+        with open(srcdir / rel_path) as rst_file:
+            referenced.update(_EXAMPLE_IMAGE_RE.findall(rst_file.read()))
+    return referenced
+
+
+def setup(app):
+    """Declare the per-project page lists and connect the checks."""
+    # Declares the `example_images` configuration value that check_example_images_used
+    # reads. Its downloader deliberately runs at a later priority than these checks.
+    app.setup_extension("_ext.example_images")
+    app.add_config_value("polar2grid_excludes", [], "env")
+    app.add_config_value("geo2grid_excludes", [], "env")
+    app.connect("config-inited", check_excluded_pages_exist)
+    app.connect("config-inited", check_example_images_used)
+    return {"version": "1.0.0", "parallel_read_safe": True, "parallel_write_safe": True}

doc/source/_ext/example_images.py

@@ -0,0 +1,63 @@
+"""Download the example images that the documentation pages reference.
+
+The images are large and are deliberately not stored in git. The list of URLs for the
+project being built is the ``example_images`` configuration value, set by ``conf.py``;
+this extension fetches whatever is not already on disk into ``_static/example_images``.
+
+See :mod:`_ext.config_checks` for the check that keeps that list and the pages that
+reference the images in agreement.
+"""
+
+import ftplib
+import os
+import urllib.request
+from pathlib import Path
+from shutil import copyfileobj
+
+from sphinx.errors import ConfigError
+from sphinx.util import logging
+
+logger = logging.getLogger(__name__)
+
+#: Run after the default priority, so `_ext.config_checks` gets to reject a stale image
+#: list before anything is fetched for it.
+DOWNLOAD_PRIORITY = 800
+
+
+def download_example_images(app, config):
+    """Download every example image for this project that is not already on disk."""
+    image_dst = Path(app.srcdir) / "_static" / "example_images"
+    image_dst.mkdir(parents=True, exist_ok=True)
+
+    for image_url in config.example_images:
+        image_pathname = image_dst / os.path.basename(image_url)
+        if image_pathname.is_file():
+            continue
+        logger.info("Downloading example image: %s", image_url)
+        if image_url.startswith(("http://", "https://")):
+            _download_http(image_url, image_pathname)
+        elif image_url.startswith("ftp://"):
+            _download_ftp(image_url, image_pathname)
+        else:
+            raise ConfigError(f"Not sure how to download image: {image_url}")
+
+
+def _download_http(image_url, image_pathname):
+    with urllib.request.urlopen(image_url) as remote_img, open(image_pathname, "wb") as local_img:
+        copyfileobj(remote_img, local_img)
+
+
+def _download_ftp(image_url, image_pathname):
+    parts = image_url.split("/")
+    server = parts[2]
+    ftp_fn = "/".join(parts[3:])
+    ftp = ftplib.FTP(server, user="ftp")  # hope for anonymous
+    with open(image_pathname, "wb") as out_file:
+        ftp.retrbinary(f"RETR {ftp_fn}", out_file.write)
+
+
+def setup(app):
+    """Declare the example image list and connect the downloader."""
+    app.add_config_value("example_images", (), "env")
+    app.connect("config-inited", download_example_images, priority=DOWNLOAD_PRIORITY)
+    return {"version": "1.0.0", "parallel_read_safe": True, "parallel_write_safe": True}

doc/source/_ext/grids_list.py

@@ -0,0 +1,100 @@
+"""Generate ``grids_list.rst`` from Polar2Grid's built-in grid definitions.
+
+The page is written into the source directory before the build reads any sources and is
+gitignored. It is listed in ``exclude_patterns`` because ``grids.rst`` pulls it in with an
+``include`` directive rather than a toctree entry, which is why this runs at a lower
+priority than the checks in :mod:`_ext.config_checks` -- those verify that every literal
+exclude entry names a file that exists.
+"""
+
+import warnings
+from pathlib import Path
+
+import yaml
+from pyproj import CRS
+
+import polar2grid
+
+#: Run ahead of the default priority so the page exists before anything looks for it.
+GENERATE_PRIORITY = 100
+
+#: The grids to document, and the title to give each one. A grid that is missing here is
+#: left out of the generated page.
+GRID_TITLES = {
+    "wgs84_fit": "WGS84 Dynamic Fit",
+    "wgs84_fit_250": "WGS84 Dynamic Fit 250m",
+    "lcc_fit": "Lambert Conic Conformal Dynamic Fit",
+    "lcc_fit_hr": "High Resolution Lambert Conic Conformal Dynamic Fit",
+    "lcc_sa": "Lambert Conic Conformal - South America Centered",
+    "lcc_eu": "Lambert Conic Conformal - Europe Centered",
+    "lcc_south_africa": "Lambert Conic Conformal - South Africa Centered",
+    "lcc_aus": "Lambert Conic Conformal - Australia Centered",
+    "lcc_asia": "Lambert Conic Conformal - Asia Centered",
+    "polar_north_pacific": "Polar-Stereographic North Pacific",
+    "polar_south_pacific": "Polar-Stereographic South Pacific",
+    "polar_alaska": "Polar-Stereographic Alaska",
+    "polar_canada": "Polar-Stereographic Canada",
+    "polar_russia": "Polar-Stereographic Russia",
+    "eqc_fit": "Equirectangular Fit",
+    "goes_east_1km": "GOES-East 1km",
+    "goes_east_4km": "GOES-East 4km",
+    "goes_east_8km": "GOES-East 8km",
+    "goes_east_10km": "GOES-East 10km",
+    "goes_west_1km": "GOES-West 1km",
+    "goes_west_4km": "GOES-West 4km",
+    "goes_west_8km": "GOES-West 8km",
+    "goes_west_10km": "GOES-West 10km",
+}
+
+
+def write_grids_list(app, config):
+    """Write ``grids_list.rst`` describing each documented built-in grid."""
+    builtin_areas_filename = Path(polar2grid.__file__).parent / "grids" / "grids.yaml"
+    with open(builtin_areas_filename) as yaml_file:
+        areas_dict = yaml.load(yaml_file, Loader=yaml.SafeLoader)
+
+    with warnings.catch_warnings(), open(Path(app.srcdir) / "grids_list.rst", "w") as grids_list_file:
+        warnings.filterwarnings("ignore", module="pyproj", category=UserWarning)
+        for area_name, area_dict in areas_dict.items():
+            area_title = GRID_TITLES.get(area_name)
+            if area_title is None:
+                continue
+            grids_list_file.write(_grid_rst(area_name, area_title, area_dict))
+
+
+def _grid_rst(area_name, area_title, area_dict):
+    """Render the reStructuredText section describing a single grid."""
+    proj = area_dict["projection"]
+    crs = CRS.from_user_input(proj.get("EPSG", proj))
+    title_underline = "^" * len(area_title)
+    rst_str = f"""
+.. _grid_{area_name}:
+
+{area_title}
+{title_underline}
+
+:Grid Name: {area_name}
+:Description: {area_dict["description"]}
+:Projection: {crs.to_string()}
+"""
+
+    if "resolution" in area_dict:
+        res = area_dict["resolution"]
+        xres = res["dx"]
+        yres = res["dy"]
+        def_units = "degrees" if crs.is_geographic else "meters"
+        units = res.get("units", def_units)
+        if xres != yres:
+            rst_str += f":Resolution (X): {xres} {units}\n"
+            rst_str += f":Resolution (Y): {yres} {units}\n"
+        else:
+            rst_str += f":Resolution: {xres} {units}\n"
+    if "area_extent" in area_dict:
+        rst_str += f":Extent: {area_dict['area_extent']}\n"
+    return rst_str
+
+
+def setup(app):
+    """Connect the generator early enough that the page exists for the rest of the build."""
+    app.connect("config-inited", write_grids_list, priority=GENERATE_PRIORITY)
+    return {"version": "1.0.0", "parallel_read_safe": True, "parallel_write_safe": True}

doc/source/toctree_filter.py → doc/source/_ext/toctree_filter.py

@@ -37,7 +37,7 @@ class TocTreeFilt(TocTree):
     """
 
     option_spec = FILTER_OPTION_SPEC
-    hasPat = re.compile(r"^\s*:(.+):(.+)$")
+    has_pat = re.compile(r"^\s*:(.+):(.+)$")
 
     # Remove any entries in the content that we dont want and strip
     # out any filter prefixes that we want but obviously don't want the
@@ -48,7 +48,7 @@ def filter_entries(self, entries):
         excl = self.state.document.settings.env.config.toc_filter_exclude
         filtered = []
         for e in entries:
-            m = self.hasPat.match(e)
+            m = self.has_pat.match(e)
             if m is not None:
                 if m.groups()[0] not in excl:
                     filtered.append(m.groups()[1])