Let a manifest declare the schema it was written against - #33
Merged
Merged
Conversation
A manifest that fails to load is diagnosed by guesswork. An unknown field has two readings, a misspelling and a field belonging to a schema this fwl-io does not implement, and the error has to offer both because nothing in the file says which applies. A model ships its manifest with its own code, so the newer-manifest case is real rather than hypothetical. An optional root `manifest_schema = <n>` settles it. A number above the schema the reader implements can only mean the reader is too old, so the manifest is refused with that alone. A number equal to it rules the newer-field reading out, so an unknown field in that manifest is reported as a misspelling and the upgrade advice is dropped. A manifest declaring an older schema keeps both readings, because the schema number rises when a manifest written for the previous one stops loading, so an unrecognised field there may be one the older schema had and this one dropped. The declaration is read before any of this code's own rules are applied, since a manifest above this schema cannot be judged by them. A table named `manifest_schema` stays an ordinary directory level, the rule `subdir` already follows, and the key written inside a table is reported as misplaced rather than misspelt, since the name is right and the level is wrong. Manifests that declare nothing are unaffected. The key is shown where an author looks: the module docstring's schema example, the add-a-dataset template, and the shipped shared manifest, which now declares it.
A manifest that declares a schema is now held to it, rather than only being checked for one that is too new. The number rises when a manifest written for the previous one stops loading, so a manifest declaring an older schema is one that has already stopped loading, and reading it on a best-effort basis only defers the failure to some individual field further in. It is refused instead, with a message naming the schema the manifest declares, the schema this code implements, and the schema versions table. Incrementing the schema is therefore a breaking change for every manifest that declares the old number. That is the intent: the declaration is what buys the sharper diagnosis, and failing at the increment with both numbers in hand is cheaper than failing later with a message about a field. A manifest that declares nothing is unaffected and still read on a best-effort basis.
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.
Closes #6 is not intended here; this addresses the manifest-schema marker discussed alongside #32.
What this does
A manifest that fails to load is currently diagnosed by guesswork. An unknown field has two readings, a misspelling and a field belonging to a schema this fwl-io does not implement, and the error has to offer both because nothing in the file says which applies. Since a model ships its manifest with its own code, the newer-manifest case is real rather than hypothetical.
An optional root
manifest_schema = <n>settles it.Why
The two-reading message was the loose end left by #32. That change made a manifest disagreeing with the installed fwl-io say so, but it could only describe the disagreement, not resolve it: without a declaration there is no way to tell a typo from a file written against a schema this code has never seen.
Changes
manifest.py: read an optional rootmanifest_schema, and thread the result through the table walk so every unknown-field message can use it.manifest_schemastays an ordinary directory level, the rulesubdiralready follows.Explanations/manifests.md, the module docstring example, and theHow-to/add_dataset.mdtemplate.data/shared_manifest.tomldeclares it, so the shipped reference example shows the intended shape.Manifests that declare nothing behave exactly as before.
Testing
macOS 15, Python 3.12. Full suite: 264 passed, 1 skipped. Ruff clean.
proteus_manifest.tomlloads unchanged (2 datasets), the shared manifest loads with its own new declaration, and entry-point discovery still returns all 3 datasets.true(rejected rather than read as 1, since Python treatsboolas anint), a table named for the key, the key written below the root, and the ordering against thesubdirrule.declared_schemafrom the grouping-level call, and from the recursion, each fails a test. Without those cases both mutations survived the whole suite.