Skip to content

feat(esm_catalog): PR-0 — package scaffold#1483

Merged
mandresm merged 9 commits into
releasefrom
esm-catalog/pr-0-scaffold
Jun 18, 2026
Merged

feat(esm_catalog): PR-0 — package scaffold#1483
mandresm merged 9 commits into
releasefrom
esm-catalog/pr-0-scaffold

Conversation

@siligam

@siligam siligam commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

PR-0 — esm_catalog package scaffold

First slice of breaking up #1473 (the 32k-line dump) into small, reviewable PRs, per @mandresm's review request. This PR is pure scaffold — no functionality yet — so the following feature PRs land on a clean, CI-tested foundation.

What's here (+102 lines, 9 files)

  • src/esm_catalog/__init__.py — package marker + __version__
  • src/esm_catalog/cli.py — minimal Click group (esm-catalog); subcommands arrive in later PRs
  • setup.pyesm-catalog console script + a catalog optional extra (pystac/shapely/universal-pathlib). No mcp extra — the MCP/LLM layer is being dropped from the product (archived on tag archive/mcp-llm)
  • src/esm_catalog/tests/test_smoke.py — import + --version smoke test
  • .github/workflows/esm-catalog-tests.yml — dedicated CI job (installs .[catalog], runs the catalog tests on Py 3.9 + 3.12)
  • setup.cfg — adds src/esm_catalog to [tool:pytest] norecursedirs so the main esm_tools suite ignores catalog tests (the dedicated job above runs them)
  • .gitignore (*.duckdb), docs/esm_catalog_index.rst stub

Notes for review

  • The catalog deps are kept behind an optional extra so they don't touch the base install.
  • A pre-existing malformed [tool.pytest] (dot) section in setup.cfg is left untouched (worth a separate cleanup; pytest already ignores it).

Context

Part of the WP1 catalogue decomposition. Full plan: decomposition design. Next: PR-A1a (STAC data-model foundation), stacked on this branch.

siligam and others added 3 commits June 13, 2026 13:56
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d setup.cfg, orphan docs stub

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@mandresm mandresm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @siligam,

Thanks for this comprehensive and short PR :)

Just a few suggestions

Comment thread .github/workflows/esm-catalog-tests.yml Outdated
Comment thread src/esm_catalog/tests/__init__.py Outdated
Comment thread src/esm_catalog/__init__.py
Comment thread setup.cfg Outdated
Comment thread setup.cfg Outdated
Comment thread setup.py
siligam added 3 commits June 17, 2026 17:25
…d CI

Move tests from src/esm_catalog/tests/ to tests/test_esm_catalog/ to
match repo convention (tests/test_<package>/, as used by esm_environment,
esm_parser, esm_viz, etc.). The scaffold smoke tests only need click,
already a base dependency, so the main pytest suite picks them up without
the isolated-install workaround — drop the now-redundant dedicated
workflow. Also collapse the dead [tool.pytest] (unrecognized by pytest in
setup.cfg) and the now-unneeded src/esm_catalog norecursedirs exclusion
into a single working [tool:pytest] section.

Per review from @mandresm on PR #1483.
The previous commit (7a910f5) removed the dedicated workflow on the
assumption that PR-0's click-only smoke tests didn't need it. But the
rest of the PR stack (A1a through C2) extends this exact workflow's
dependency-install line incrementally (xarray/netCDF4, cfgrib, duckdb,
fastapi, pytz, ...) — it's the only thing running esm_catalog tests in
CI for the whole stack, since the base install_requires pins
ruamel.yaml.clib==0.2.7, which fails to build on Python 3.12, and the
main esm-tools CI matrix only covers 3.8-3.11. Restoring it, repointed
at tests/test_esm_catalog (and adding that path to the trigger filters).

Also restore the tests/test_esm_catalog exclusion in [tool:pytest]
norecursedirs (just relocated from src/esm_catalog) — once downstream
PRs add pystac/xarray-importing tests there, the main suite's
`pytest -v .` step (which doesn't install catalog extras) would
otherwise fail to collect them.
…t scan

The main esm-tools CI workflow runs `pytest --doctest-modules src`,
which collects and executes doctests from every source module, not
just tests/. The original norecursedirs entry excluded all of
src/esm_catalog (not just its old tests/ subdir), which incidentally
also kept the main suite from executing src/esm_catalog/uri.py's
doctest example — that example imports universal-pathlib, an extras-
only dependency the main suite doesn't install. Dropping that
exclusion (in 7a910f5) broke `basic_tests` on every PR from A1a
onward. Restoring it alongside the relocated tests/test_esm_catalog
exclusion, matching the actual end-state norecursedirs.
@siligam

siligam commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Summary of how the review feedback above landed, since the back-and-forth in the threads (a couple of which I had to self-correct) is easier to follow as one note:

Tests relocated — moved from src/esm_catalog/tests/ to tests/test_esm_catalog/, matching this repo's tests/test_<package>/ convention (test_esm_environment, test_esm_parser, test_esm_viz, ...).

Dedicated esm-catalog-tests.yml workflow — kept, repointed at the new path. Initially removed it per the suggestion above, since PR-0's own smoke tests don't need it. But while rebasing the rest of this PR stack onto the fix, I found 6 of the 12 follow-on PRs extend this exact workflow's dependency-install line as the catalog grows (xarray/netCDF4, cfgrib, duckdb, the FastAPI stack, pytz...) — it's the only thing running catalog tests in CI for the whole stack, since base install_requires pins ruamel.yaml.clib==0.2.7 (fails to build on 3.12) and the main CI matrix only covers 3.8–3.11. Restored it.

setup.cfg — collapsed the dead [tool.pytest] (dot, not recognized by pytest) into one working [tool:pytest] section, with norecursedirs = tests/helpers src/esm_catalog tests/test_esm_catalog. Both src/esm_catalog and the relocated test dir need to stay excluded from the main suite: the main esm-tools workflow runs pytest --doctest-modules src, which executes doctests embedded in source-file docstrings (src/esm_catalog/uri.py has one that needs universal-pathlib, an extras-only dep) — dropping that exclusion broke basic_tests for a bit.

Catalog deps stay under the catalog extra (not folded into base install_requires) — the catalog is still mid-rollout across this PR stack (~13 PRs), so I'd rather not add pystac/shapely/etc. to every existing esm-tools install until it's complete. Open to revisiting once the stack lands.

Also fixed while rebasing: a pre-existing bug surfaced on Python 3.9 — src/esm_catalog/api/responses.py (added in PR-B1, #1490) uses X | None union syntax that pydantic v2 can't resolve on Python <3.10 without eval_type_backport. Latent since B1, it only started failing CI once PR-B3 wired it into the import chain. Added eval_type_backport as a dependency (pydantic's own recommended fix) rather than rewriting the ~34 files that use this syntax.

All 13 PRs in the stack (#1483#1496) have been rebased onto the corrected base and are green: 162 catalog tests passing on both Python 3.9 and 3.12, plus the full main esm-tools suite.

@mandresm

Copy link
Copy Markdown
Contributor

Thanks for the summary. I'm going to have a look at it now.

@mandresm mandresm mentioned this pull request Jun 18, 2026
3 tasks

@mandresm mandresm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one minor suggestion. Thanks for the changes and the clarifications!

Comment thread setup.cfg
Comment thread setup.cfg Outdated
Comment thread setup.py
@mandresm

Copy link
Copy Markdown
Contributor

Hi @siligam, this is ready to be merged, but before merging it needs bumping and tests to finish. We have a bumping workflow that you'll see me trigger below.

I am going to bump the version as a patch. It is not a patch but is not either a full feature so that's why I'll be using patch.

@mandresm

Copy link
Copy Markdown
Contributor

#bump #patch

@mandresm
mandresm merged commit 72019da into release Jun 18, 2026
12 checks passed
@mandresm
mandresm deleted the esm-catalog/pr-0-scaffold branch June 18, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants