manifest-schema.yml: say what the validator honours today - #140
Merged
Conversation
The `schema` block header said the validator reads the raw file "honouring `delimiter` ... and `read_as` (positional workbook reads)". `read_raw()` in `builders/_validate.py` honours `delimiter` only: it consults `schema.format`, `schema.delimiter` (falling back to a top-level `delimiter`) and the `datetime64` column names, passes exactly `sep` and `parse_dates` to `read_csv`, and raises `NotImplementedError` for any format but `csv`. The identifier `read_as` does not appear in the module. No CSV manifest carries `read_as`, so nothing misreads today — the sentence promised a reader that does not exist yet. Reworded to describe the validator as shipped, and to say what `read_as` is for: the contract for workbook range reads once those are built, which is what leaves the 13 non-CSV manifests on the conformance pass alone. Also, beyond the issue's scope but in the same family — comments that state a count and have drifted from the corpus they describe: `source.doi` said "in use by 16 of 33 manifests" and `source.version` "in use by 10". Counted with PyYAML over the 44 sidecars on main: 17 and 21. Comment-only; no field changes. `scripts/validate_datasets.py` still reports 44 manifest(s): 44 pass, 0 fail under both legs of the workflow matrix (pandas 2.3.3 and 3.0.5), and the `--builders` layer is green. Closes #137. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The change is documentation-only and the updated statements match the current validator implementation and manifest corpus counts.
Pull request overview
Updates manifest-schema.yml documentation to accurately describe what the current manifest-driven validator actually reads/honours today, avoiding promises of reader behavior that is not implemented.
Changes:
- Corrects the
schemablock header comment to matchbuilders/_validate.py:read_raw()behavior (CSV-only,delimiter,parse_datesfordatetime64, noindex_col). - Updates the “in use by …” counts for
source.doiandsource.versionto reflect the current manifest corpus.
File summaries
| File | Description |
|---|---|
| manifest-schema.yml | Documentation-only corrections to align the schema header/comments with the shipped validator behavior and current manifest usage counts. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
schemablock header promised a reader that does not exist yet. It said the validator reads the raw file "honouringdelimiter(countries.csv is semicolon-separated) andread_as(positional workbook reads)". Surfaced as the doc nit in the first verification comment on #127.What
read_raw()actually doesbuilders/_validate.pylines 121-131. It consults exactly three manifest keys —schema.format(a gate: any value butcsvraises),schema.delimiterwith a fallback to a top-leveldelimiter, and thedatetime64column names viadatetime_columns()— and passes exactly two kwargs toread_csv,sepandparse_dates. Noindex_col, nokeep_default_na, nodtype, noheader, nosheet_name. The identifierread_asdoes not occur anywhere in the module.So
read_asis honoured by nothing today. It does not misread anything either, because no CSV manifest carries it — the 13 manifests that do are the non-CSV ones, which take the conformance pass alone and never reachread_raw(). The sentence was describing a validator that has not been built.Reworded to describe the validator as shipped, and to say what
read_asis for: the contract for workbook range reads once those exist, which is precisely why the non-CSV manifests are conformance-only.Beyond the issue's scope, flagged rather than slipped in
Two comments in the same file state a count and have drifted from the corpus they describe. Counted with PyYAML over the 44 sidecars on
main:source.doisource.versionSame class of defect as the sentence above — a document describing the repository incorrectly — and one line each, so they are fixed here rather than left to drift further. Say the word if you would rather they went to their own issue.
Verification
Comment-only; no field changes, no data bytes.
scripts/validate_datasets.pyreports44 manifest(s): 44 pass, 0 fail; bytes-validated formats: csv; conformance-only: {'xlsx': 5, 'npy': 2, 'dta': 4, 'json': 1, 'xls': 1}under both legs of the workflow matrix (pandas 2.3.3 and 3.0.5), and--buildersreportsbuilder layer: all green.Closes #137.
🤖 Generated with Claude Code