Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions .github/workflows/validate-datasets.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,15 @@
# consumed-file-check.yml stays beside this as the byte-integrity (sha256) and
# catalog-freshness gate; this job is the schema half PLAN Phase 5 promised.
#
# pandas is pinned to requirements.txt's version (2.3.3), deliberately one
# major behind the lectures' anaconda=2026.07 (pandas 3): the validator
# compares dtypes by family precisely so it is green on both, and running the
# older one here is the standing check that it stays so.
# Runs as a matrix over BOTH pandas majors -- requirements.txt's 2.3.3 (the
# canary's environment) and 3.0.5 (what anaconda=2026.07, the lectures' pin,
# carries; bump it when that pin moves, never float it: a red here must be
# reproducible a week later) -- and runs
# BOTH validation layers: the shared manifest-driven one, and each dynamic
# builder's own validate() on its committed bytes (`--builders`, no network).
# The 2.3.3-only, shared-layer-only first version claimed to be "the standing
# check that it stays green on both" and was not: business_cycle.py's band
# check failed under pandas 3 for a week without anything noticing (#128).

name: validate-datasets

Expand All @@ -31,12 +36,21 @@ on:
jobs:
validate:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
pandas: ["2.3.3", "3.0.5"]
name: validate (pandas ${{ matrix.pandas }})
steps:
- uses: actions/checkout@v4
with:
lfs: false # lectures/ is plain git; a pointer here should fail (see consumed-file-check.yml)
- uses: actions/setup-python@v5
with:
python-version: "3.12"
- run: pip install pandas==2.3.3 PyYAML==6.0.3
# wbgapi: builders/business_cycle.py imports it at module level, and the
# builder layer imports the module (it never calls the network here).
- run: pip install pandas==${{ matrix.pandas }} PyYAML==6.0.3 wbgapi==1.0.12
- run: python -c "import pandas; print('pandas', pandas.__version__)"
Comment thread
mmcky marked this conversation as resolved.
- run: python scripts/validate_datasets.py
- run: python scripts/validate_datasets.py --builders
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ Where one builder produces a **set** of files, name it for the set and let each

**Where a builder reads its input from.** The normal case is the third-party upstream, fetched at run time: eight of the nine `committed` builders here do that, and it is the fetch stage of the contract below. A builder reads from `sources/` **only when the input cannot be re-fetched** — the upstream is gone, unlocatable, or was inherited with no recoverable source. `sources/` is that exception layer, not a general input tree, and it is emphatically not "the big-file directory": the defining property is un-refetchability, not size. What it must never be is a network read from another QuantEcon repo — that is how a retired repo becomes load-bearing again.

Builders follow four stages — **fetch → pre-process → validate → write** — and only write on validation pass. **The validate stage is shared**: `builders/_validate.py` reads the manifest's `schema` block as its spec (columns and `pattern` runs, dtype families, exact `known_nulls`, the `nulls:` placement rule, `row_count_floor`, `date_range`) and measures the overlap window against the previous vintage; a builder calls `validate(frame.reset_index(), manifest, previous)` and layers on only what the schema cannot say — value bands, a grid check, the per-series revision **bound** (a tracking snapshot is revised by its source, so the test is a tolerance plus a printed summary, never equality). The same function runs over every committed CSV on every PR (`scripts/validate_datasets.py`, `validate-datasets.yml`), so a manifest that drifts from its bytes fails the PR, not the next refresh. Lectures always read the last-good snapshot: an upstream outage may fail a refresh, it must never break a lecture build.
Builders follow four stages — **fetch → pre-process → validate → write** — and only write on validation pass. **The validate stage is shared**: `builders/_validate.py` reads the manifest's `schema` block as its spec (columns and `pattern` runs, dtype families, exact `known_nulls`, the `nulls:` placement rule, `row_count_floor`, `date_range`) and measures the overlap window against the previous vintage; a builder calls `validate(frame.reset_index(), manifest, previous)` and layers on only what the schema cannot say — value bands, a grid check, the per-series revision **bound** (a tracking snapshot is revised by its source, so the test is a tolerance plus a printed summary, never equality). The same function runs over every committed CSV on every PR (`scripts/validate_datasets.py`, `validate-datasets.yml`), so a manifest that drifts from its bytes fails the PR, not the next refresh. **A dynamic builder also exposes `check_committed()`** — its own `validate()` on the committed bytes, no network — which the same workflow runs (`--builders`) under both pandas majors; a builder-specific check that breaks on a pandas change fails the PR that introduces it (#128 was a week-old pandas-3 break the shared layer could not see). Lectures always read the last-good snapshot: an upstream outage may fail a refresh, it must never break a lecture build.

**One builder per source, composite files where a lecture reads series together, shared fetch libraries** (decided 2026-09-01, #26): `builders/_fred.py` is the `Fred` class for FRED reads — no `pandas_datareader` in a builder — and a lecture's FRED data is one file with the lecture's own variable names as columns, not six. A builder that writes a set validates every file before writing any.

Expand Down
4 changes: 3 additions & 1 deletion builders/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ and measures the overlap window against the previous vintage
calls `validate(frame.reset_index(), manifest, previous)` and adds only what
a schema cannot say — value bands, a grid, recency, the revision *bound*. The
same function runs over every committed CSV on every PR
(`scripts/validate_datasets.py`, `validate-datasets.yml`). One
(`scripts/validate_datasets.py`, `validate-datasets.yml`), and each dynamic
builder's `check_committed()` runs its own layer there too, under both pandas
majors, with no network. One
builder per **source** for a lecture's data, writing a composite file where
the lecture reads the series together (decided 2026-09-01 on #26).

Expand Down
15 changes: 14 additions & 1 deletion builders/_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ def validate(frame, previous=None):
raw = frame.reset_index() if frame.index.name else frame
prev_raw = previous.reset_index() if previous is not None and previous.index.name else previous
shared = validate_schema(raw, manifest, prev_raw)
# builder-specific: bands / grid / recency ...
# builder-specific: bands / grid / recency ... Two idioms that bit once:
# call .dropna() BEFORE a band check (pandas 3's stack() keeps NaN, #128),
# and never compare against a dtype string -- the shared validator already
# did that by family (#122).
raise NotImplementedError
summary = {
'dataset': OUT_FILE,
Expand All @@ -101,6 +104,16 @@ def validate(frame, previous=None):
return summary


def check_committed():
"""Builder-layer validation of the COMMITTED file(s), no network. Called by
scripts/validate_datasets.py --builders on every PR, under both pandas
majors, so a builder-specific check that breaks on a pandas change fails
the PR rather than the next canary. Yield each file validated."""
frame = pd.read_csv(os.path.join(PUBLISHED_DIR, OUT_FILE), index_col=0)
validate(frame)
yield OUT_FILE


def _atomic_write(path, text):
tmp = path + '.tmp'
with open(tmp, 'w') as f:
Expand Down
16 changes: 15 additions & 1 deletion builders/business_cycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ def validate(table, frame, previous=None):
values = frame[year_cols]
_check(values.abs().max().max() >= table['min_abs_max'], f'{name}: values look like ratios')
lo, hi = table['band']
_check(values.stack().between(lo, hi).all(), f'{name}: value out of band [{lo}, {hi}]')
# .dropna() is load-bearing: pandas 3's stack() keeps NaN (no dropna= any
# more), and the structural nulls the manifest places would fail between().
# Found by the #127 validation (#128).
_check(values.stack().dropna().between(lo, hi).all(), f'{name}: value out of band [{lo}, {hi}]')
_check(years[-1] >= dt.date.today().year - MAX_STALENESS_YEARS, f'{name}: newest year is {years[-1]}')
if table['first_year_null']:
_check(values[year_cols[0]].isnull().all(), f'{name}: a value in {year_cols[0]} -- growth is undefined in the first year')
Expand All @@ -167,6 +170,17 @@ def validate(table, frame, previous=None):
return summary


def check_committed():
"""Run this builder's validate() on the COMMITTED files, no network — the
builder-layer half of PR validation (scripts/validate_datasets.py
--builders), so a pandas or builder change that would fail the next
canary fails the PR that introduces it instead (#128)."""
for table in TABLES:
frame = pd.read_csv(os.path.join(PUBLISHED_DIR, table['file']), index_col=0)
validate(table, frame)
yield table['file']


def _atomic_write(path, text):
tmp = path + '.tmp'
with open(tmp, 'w') as f:
Expand Down
8 changes: 8 additions & 0 deletions builders/business_cycle_fred.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ def validate(frame, previous=None):
return summary


def check_committed():
"""Builder-layer validation of the COMMITTED file, no network (see
scripts/validate_datasets.py --builders)."""
frame = pd.read_csv(os.path.join(PUBLISHED_DIR, OUT_FILE), index_col=0, parse_dates=True)
validate(frame)
yield OUT_FILE


def _atomic_write(path, text):
tmp = path + '.tmp'
with open(tmp, 'w') as f:
Expand Down
49 changes: 48 additions & 1 deletion scripts/validate_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

python scripts/validate_datasets.py # everything under lectures/
python scripts/validate_datasets.py lectures/gdp_growth_annual.csv.yml ...
python scripts/validate_datasets.py --builders # each dynamic builder's own
# validate() on the committed bytes

Two passes per manifest:

Expand Down Expand Up @@ -91,8 +93,53 @@ def check_columns(cols, where):
return errs


def builder_layer() -> int:
"""The builder-layer half: every dynamic snapshot's own validate() on its
COMMITTED bytes, no network, through the builder's check_committed()
(#128 -- a builder check that broke under pandas 3 while the shared
layer stayed green, unseen until a validator dry-ran the builder)."""
import importlib
failed = 0
seen = set() # a set-writing builder is named by several manifests; run it once
for path in sorted(LECTURES.glob('*.yml')):
m = yaml.safe_load(path.read_text(encoding='utf-8')) or {}
if m.get('class') != 'dynamic-snapshot' or m.get('builder_status') != 'committed':
continue
# A malformed manifest (no builder, no filename) is annotated on the
# sidecar, never a bare KeyError -- the conformance pass catches it
# first in CI, but --builders also runs on its own (Copilot on #131).
builder, filename = m.get('builder'), m.get('filename') or path.name[:-4]
sidecar = path.relative_to(REPO)
if not builder:
failed += 1
print(f'::error file={sidecar}::dynamic snapshot with builder_status committed but no builder path')
print(f'FAIL {filename}: builder layer')
continue
if builder in seen:
continue
seen.add(builder)
try:
mod = importlib.import_module(pathlib.Path(builder).stem)
fn = getattr(mod, 'check_committed', None)
if fn is None:
raise AttributeError(f'{builder} has no check_committed() (see builders/_template.py)')
for f in fn():
print(f'{"ok builder layer":70s} {f} ({builder})')
except Exception as e:
failed += 1
print(f'::error file={builder}::{type(e).__name__}: {e}')
print(f'FAIL {filename}: builder layer')
return failed


def main(argv: list[str]) -> int:
paths = [pathlib.Path(a) for a in argv] or sorted(LECTURES.glob('*.yml'))
if argv and argv[0] == '--builders':
n = builder_layer()
print(f'\nbuilder layer: {"all green" if not n else f"{n} builder(s) failed"}')
return 1 if n else 0
# Resolve the arguments: REPO is absolute and relative_to() does not
# resolve, so a relative path crashed the failure report (#129).
paths = [pathlib.Path(a).resolve() for a in argv] or sorted(LECTURES.glob('*.yml'))
failed = 0
skipped_formats: dict[str, int] = {}
for path in paths:
Expand Down
Loading