From f2e766cdd76432e3c6c29cd6fa62d156f3bb21f3 Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Sun, 26 Jul 2026 11:56:44 +0200 Subject: [PATCH 1/2] Let a manifest declare the schema it was written against 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 = ` 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. --- docs/Explanations/manifests.md | 3 +- docs/How-to/add_dataset.md | 4 ++ src/fwl_io/data/shared_manifest.toml | 6 +- src/fwl_io/manifest.py | 103 ++++++++++++++++++++++++--- tests/test_manifest.py | 94 ++++++++++++++++++++++++ 5 files changed, 199 insertions(+), 11 deletions(-) diff --git a/docs/Explanations/manifests.md b/docs/Explanations/manifests.md index 95b8b46..a50bfc7 100644 --- a/docs/Explanations/manifests.md +++ b/docs/Explanations/manifests.md @@ -25,6 +25,7 @@ Validation at load time: - `extract`, when present, must be `"tar"` or `"zip"`. - A dataset table must not contain sub-tables, and arrays of tables are rejected; ambiguous structures fail loudly instead of being silently dropped. - A dataset table declares only the fields above, and a grouping level declares none: anything else raises `ManifestSchemaError`, naming the field and the manifest schema this fwl-io implements. A model ships its manifest with its own code, so a manifest can be newer than the installed fwl-io; ignoring an unknown field silently would leave the manifest asking for something it never gets, and a `required_by` written one level above its dataset would leave the dataset claiming no model needs it. The message names the action that fits the case: move a dataset field that sits too high, delete a field this fwl-io no longer takes, and for a name it does not know at all, check the spelling or upgrade. Two things are outside the check. A scalar at the manifest root is reserved for a manifest's own settings and is ignored, unless it names a dataset field or `subdir`. And a table is recognised as a dataset by its `zenodo` key, so a misspelt `zenodo` is reported as a table with no pin rather than as an unknown field. +- A manifest may declare the schema it was written against with a root `manifest_schema = `. It is optional, and declaring it sharpens the diagnosis: a number above the schema the installed fwl-io implements can only mean the reader is too old, so the error says to upgrade rather than offering the spelling reading as well. A number the reader does implement rules the upgrade reading out, so an unknown field in that manifest is reported as a misspelling alone. The value must be a whole number of at least 1; `true` is rejected rather than read as 1. - A manifest that fails to load takes its whole provider with it: `discover_manifests` skips that package and logs a warning, so its other datasets disappear from the result too. Use `fwl-io list` to see the error. - A `subdir` field is rejected on any table, dataset or grouping level, and at the manifest root: the location comes from the key. A table *named* `subdir` is an ordinary directory level. - Within one manifest, two keys that differ only in case are rejected: they would share one directory and one registry file on a case-insensitive filesystem. Two installed packages declaring keys that collide is a separate check, tracked in [#18](https://github.com/FormingWorlds/fwl-io/issues/18). @@ -35,7 +36,7 @@ Validation at load time: |---|---|---| | 1 | 26.7.22 | A dataset's location is derived from its dotted table key; `subdir` is not a field. | -An error raised while reading a manifest names the schema the running code implements, so a mismatch between a manifest and an installed fwl-io can be placed against this table. +An error raised while reading a manifest names the schema the running code implements, so a mismatch between a manifest and an installed fwl-io can be placed against this table. A manifest that declares `manifest_schema` is checked against it directly. ## Archive datasets diff --git a/docs/How-to/add_dataset.md b/docs/How-to/add_dataset.md index cc3af9a..02280d7 100644 --- a/docs/How-to/add_dataset.md +++ b/docs/How-to/add_dataset.md @@ -20,12 +20,16 @@ Choose the manifest: Add a table for the dataset: ```toml +manifest_schema = 1 + [interior.eos.wolf_bower_2018] name = "Wolf & Bower (2018) MgSiO3 equation of state" zenodo = "10.5281/zenodo.1234567" required_by = ["aragog", "zalmoxis", "spider"] ``` +The root `manifest_schema` names the schema the file is written against. It is optional and worth declaring: it lets fwl-io tell a manifest written for a newer schema from a misspelt field, so a load failure names the one that applies instead of offering both. The current schema is in the [schema versions](../Explanations/manifests.md#schema-versions) table. + The dotted key is the location below `FWL_DATA`, so this dataset lands in `interior/eos/wolf_bower_2018/r`, the version directory named for its Zenodo record. Choose the key to follow the [target layout](../Explanations/manifests.md#the-fwl_data-layout), using only letters, digits, `_` and `-` per segment, each starting with a letter, digit or `_`. `required_by` lists the models whose `fwl-io fetch ` should include this dataset. If the deposit is a single archive that consumers expect unpacked, add `extract = "tar"` or `extract = "zip"`; the archive is downloaded, checksum-verified, and unpacked into the dataset directory. See [Archive datasets](../Explanations/manifests.md#archive-datasets). diff --git a/src/fwl_io/data/shared_manifest.toml b/src/fwl_io/data/shared_manifest.toml index 821ad41..8b0852d 100644 --- a/src/fwl_io/data/shared_manifest.toml +++ b/src/fwl_io/data/shared_manifest.toml @@ -1,6 +1,8 @@ # Datasets shared by several models of the PROTEUS ecosystem. # -# One TOML table per dataset: +# One TOML table per dataset, below an optional schema declaration: +# +# manifest_schema = 1 # # [group.dataset_key] # name = "Human-readable dataset name" @@ -23,3 +25,5 @@ # No dataset is shared across several models yet, so this manifest declares # none. The Baraffe stellar tracks ship with the MORS package, which owns their # manifest and registry. + +manifest_schema = 1 diff --git a/src/fwl_io/manifest.py b/src/fwl_io/manifest.py index 0c45dba..fe64db0 100644 --- a/src/fwl_io/manifest.py +++ b/src/fwl_io/manifest.py @@ -8,6 +8,8 @@ Manifest schema, one table per dataset, identified by its ``zenodo`` key:: + manifest_schema = 1 # optional, see below + [interior.eos.wolf_bower_2018] name = "Wolf & Bower (2018) MgSiO3 equation of state" zenodo = "10.5281/zenodo.1234567" # version DOI, never a concept DOI @@ -15,6 +17,12 @@ required_by = ["aragog", "zalmoxis", "spider"] extract = "tar" # optional: unpack a single-archive deposit +The optional root ``manifest_schema`` names the schema the file was written +against. Declaring it sharpens the diagnosis when something does not load: a +number above the schema the installed fwl-io implements can only mean the +reader is too old, and a number equal to it rules that reading out, so an +unknown field is reported as a misspelling alone. + The dotted table key is the dataset location below the data root: the table above resolves into ``interior/eos/wolf_bower_2018``. Key segments are restricted to letters, digits, ``_`` and ``-``, each starting with a letter, @@ -79,6 +87,12 @@ # checkout's recorded version does not. _MANIFEST_SCHEMA = 1 +# The root key by which a manifest states the schema it was written against. +# Declaring it is optional and turns an ambiguous diagnosis into a definite +# one: a manifest that names a schema this code does not implement is from the +# future and says so, rather than being reported as a possible typo. +_SCHEMA_KEY = 'manifest_schema' + class ManifestSchemaError(ValueError): """A manifest and the installed fwl-io disagree about the manifest schema. @@ -101,18 +115,75 @@ def _reading_version() -> str: return f'manifest schema {_MANIFEST_SCHEMA} (distribution {distribution})' -def _unknown_field_error(what: str) -> ManifestSchemaError: +def _unknown_field_error(what: str, declared_schema: int | None = None) -> ManifestSchemaError: """Build the error for a field this fwl-io does not know. - Both readings are offered because both are common: a misspelt field, and a - manifest written against a schema newer than the fwl-io reading it. + Without a declared schema both readings are offered, because both are + common: a misspelt field, and a manifest written against a schema newer + than the fwl-io reading it. A manifest declaring the schema this code + implements has ruled the second reading out, so the message names the typo + alone. A manifest declaring an older schema does not: the schema number + rises whenever a manifest written for the previous one stops loading, so a + field this code does not know may be one that older schema had and this + one dropped, and both readings stay on the table. """ + if declared_schema == _MANIFEST_SCHEMA: + return ManifestSchemaError( + f'{what}. The manifest declares {_SCHEMA_KEY} {declared_schema}, the schema ' + f'this fwl-io implements, so the field is misspelt rather than newer than ' + f'this code.' + ) return ManifestSchemaError( f'{what}. This fwl-io reads {_reading_version()}: check the spelling, or ' f'upgrade fwl-io if the manifest was written against a newer schema.' ) +def _read_declared_schema(tree: dict) -> int | None: + """Return the schema a manifest declares at its root, if it declares one. + + Raises when the manifest names a schema this code does not implement, + which is the case the key exists to make unambiguous, and when the value + is not a schema number at all. + """ + if _SCHEMA_KEY not in tree: + return None + declared = tree[_SCHEMA_KEY] + # A table of this name is a directory level like any other, the same rule + # `subdir` follows: the reserved name applies to the scalar, not to a + # dataset an author happens to have called this. Leave it to the walk. + if isinstance(declared, dict) or ( + isinstance(declared, list) and any(isinstance(item, dict) for item in declared) + ): + return None + # bool is an int subclass, and `manifest_schema = true` is a mistake worth + # naming rather than reading as schema 1. + if isinstance(declared, bool) or not isinstance(declared, int) or declared < 1: + raise ManifestSchemaError( + f'the manifest root declares {_SCHEMA_KEY} {declared!r}; it must be a whole ' + f'number of at least 1, naming the manifest schema the file was written ' + f'against.' + ) + if declared > _MANIFEST_SCHEMA: + raise ManifestSchemaError( + f'the manifest declares {_SCHEMA_KEY} {declared}, but this fwl-io reads ' + f'{_reading_version()}: upgrade fwl-io to read this manifest.' + ) + return declared + + +def _misplaced_schema_error(what: str) -> ManifestSchemaError: + """Build the error for the reserved schema key written below the root. + + The name is spelt correctly and sits at the wrong level, so reporting it as + an unknown field would give the one reading that is certainly wrong. + """ + return ManifestSchemaError( + f'{what}. {_SCHEMA_KEY!r} is a manifest-root key naming the schema the file was ' + f'written against: move the line above the first table.' + ) + + def _misplaced_field_error(what: str) -> ManifestSchemaError: """Build the error for a known field declared outside a dataset table.""" return ManifestSchemaError( @@ -195,7 +266,9 @@ def _reject_declared_subdir(where: str, table: dict, derived: str | None = None) ) -def _walk_tables(tree: dict, prefix: str = '') -> list[tuple[str, dict]]: +def _walk_tables( + tree: dict, prefix: str = '', declared_schema: int | None = None +) -> list[tuple[str, dict]]: """Return (dotted-key, table) pairs for the dataset tables of a manifest. A table is a dataset when it carries the ``zenodo`` key. Grouping tables @@ -217,12 +290,18 @@ def _walk_tables(tree: dict, prefix: str = '') -> list[tuple[str, dict]]: ) raise _misplaced_field_error(where) if prefix: + if name == _SCHEMA_KEY: + raise _misplaced_schema_error( + f'grouping table {prefix.rstrip(".")!r} declares {name!r}' + ) raise _unknown_field_error( f'grouping table {prefix.rstrip(".")!r} declares {name!r}, and a ' - f'grouping level takes no fields' + f'grouping level takes no fields', + declared_schema, ) # A root scalar that names no dataset field is a manifest's own - # setting. + # setting. The schema declaration is one of those, already read + # and validated before the walk began. continue dotted = f'{prefix}{name}' _validate_key_segment(name, prefix.rstrip('.')) @@ -237,15 +316,18 @@ def _walk_tables(tree: dict, prefix: str = '') -> list[tuple[str, dict]]: if has_subtables: raise ValueError(f'dataset {dotted!r}: dataset tables must not contain sub-tables') unknown = sorted(set(value) - _DATASET_FIELDS) + if _SCHEMA_KEY in unknown: + raise _misplaced_schema_error(f'dataset {dotted!r} declares {_SCHEMA_KEY!r}') if unknown: raise _unknown_field_error( f'dataset {dotted!r} declares {", ".join(repr(f) for f in unknown)}, ' f'which is not a dataset field ' - f'(known fields: {", ".join(sorted(_DATASET_FIELDS))})' + f'(known fields: {", ".join(sorted(_DATASET_FIELDS))})', + declared_schema, ) leaves.append((dotted, value)) elif has_subtables: - leaves.extend(_walk_tables(value, prefix=f'{dotted}.')) + leaves.extend(_walk_tables(value, prefix=f'{dotted}.', declared_schema=declared_schema)) else: raise ValueError( f'table {dotted!r} has no "zenodo" key, so it is neither a dataset nor a ' @@ -261,10 +343,13 @@ def load_manifest(path: str | Path) -> list[Dataset]: with path.open('rb') as fh: tree = tomllib.load(fh) + # Read the declaration first: a manifest above this code's schema cannot be + # judged by this code's rules, so it must be refused before they are applied. + declared_schema = _read_declared_schema(tree) _reject_declared_subdir('the manifest root', tree) datasets: list[Dataset] = [] folded: dict[str, str] = {} - for key, table in _walk_tables(tree): + for key, table in _walk_tables(tree, declared_schema=declared_schema): clash = folded.setdefault(key.lower(), key) if clash != key: raise ValueError( diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 824a638..8f89f92 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -783,3 +783,97 @@ def test_dataset_field_at_the_manifest_root_is_rejected(tmp_path): # name and not about scalars at the root. setting = 'schema_version = 1\n[demo]\nzenodo = "10.5281/zenodo.1"\n' assert load_manifest(_write(tmp_path, setting))[0].key == 'demo' + + +DEMO = '[demo]\nzenodo = "10.5281/zenodo.1"\n' + + +def test_a_manifest_from_a_newer_schema_says_so(tmp_path): + """A schema above the implemented one can only mean the reader is too old.""" + with pytest.raises(ManifestSchemaError, match='upgrade fwl-io') as excinfo: + load_manifest(_write(tmp_path, f'manifest_schema = 99\n{DEMO}')) + # The spelling reading is dropped: nothing about this file is misspelt. + assert 'spelling' not in str(excinfo.value) + # The same file at the implemented schema loads, so the refusal is the + # number's doing and not the declaration's presence. + assert load_manifest(_write(tmp_path, f'manifest_schema = 1\n{DEMO}'))[0].key == 'demo' + + +def test_declaring_the_implemented_schema_makes_an_unknown_field_a_typo(tmp_path): + """Declaring the schema this code implements rules out the newer-field reading.""" + body = '[demo]\nzenodo = "10.5281/zenodo.1"\nrequired_bye = ["proteus"]\n' + with pytest.raises(ManifestSchemaError, match='misspelt') as declared: + load_manifest(_write(tmp_path, f'manifest_schema = 1\n{body}')) + assert 'upgrade fwl-io' not in str(declared.value) + # Without the declaration the same file still offers both readings, so the + # sharper message is attributable to the declaration. + with pytest.raises(ManifestSchemaError, match='upgrade fwl-io'): + load_manifest(_write(tmp_path, body)) + + +def test_the_sharper_message_reaches_nested_and_grouping_tables(tmp_path): + """The declaration sharpens every unknown-field message, at any depth.""" + nested = 'manifest_schema = 1\n[grp.demo]\nzenodo = "10.5281/zenodo.1"\nrequired_bye = ["p"]\n' + with pytest.raises(ManifestSchemaError, match='misspelt') as deep: + load_manifest(_write(tmp_path, nested)) + assert 'upgrade fwl-io' not in str(deep.value), ( + 'a dataset below a grouping level lost the declaration on the way down' + ) + + grouping = ( + 'manifest_schema = 1\n[grp]\nnickname = "x"\n[grp.demo]\nzenodo = "10.5281/zenodo.1"\n' + ) + with pytest.raises(ManifestSchemaError, match='misspelt') as level: + load_manifest(_write(tmp_path, grouping)) + assert 'upgrade fwl-io' not in str(level.value), 'a grouping level lost the declaration' + + +@pytest.mark.parametrize( + 'value', + ['0', '-1', '"1"', '1.5', 'true'], + ids=['zero', 'negative', 'text', 'fractional', 'boolean'], +) +def test_the_schema_declaration_must_be_a_schema_number(tmp_path, value): + """Anything that is not a positive whole number is a mistake, including true.""" + with pytest.raises(ManifestSchemaError, match='whole number') as excinfo: + load_manifest(_write(tmp_path, f'manifest_schema = {value}\n{DEMO}')) + assert 'manifest_schema' in str(excinfo.value) + + +def test_a_table_named_for_the_schema_key_is_an_ordinary_directory(tmp_path): + """The reserved name applies to the scalar, as with subdir, not to a table.""" + dataset = '[manifest_schema]\nzenodo = "10.5281/zenodo.1"\n' + assert load_manifest(_write(tmp_path, dataset))[0].key == 'manifest_schema' + # Also as a grouping level, where the walk has to recurse past it. + nested = '[manifest_schema.demo]\nzenodo = "10.5281/zenodo.1"\n' + assert load_manifest(_write(tmp_path, nested))[0].key == 'manifest_schema.demo' + + +def test_the_schema_key_below_the_root_is_misplaced_rather_than_misspelt(tmp_path): + """The reserved key inside a table is at the wrong level, not spelt wrong.""" + in_dataset = f'{DEMO}manifest_schema = 1\n' + with pytest.raises(ManifestSchemaError, match='move the line above the first table'): + load_manifest(_write(tmp_path, in_dataset)) + + in_grouping = ( + 'manifest_schema = 1\n[grp]\nmanifest_schema = 1\n[grp.demo]\nzenodo = "10.5281/zenodo.1"\n' + ) + with pytest.raises(ManifestSchemaError, match='move the line above the first table') as grp: + load_manifest(_write(tmp_path, in_grouping)) + assert 'misspelt' not in str(grp.value) + + +def test_a_future_manifest_is_refused_before_this_codes_own_rules(tmp_path): + """A manifest above this schema cannot be judged by this schema's rules.""" + future = f'manifest_schema = 99\nsubdir = "x"\n{DEMO}' + with pytest.raises(ManifestSchemaError, match='upgrade fwl-io') as excinfo: + load_manifest(_write(tmp_path, future)) + # The subdir rule is one of this code's rules, so it must not be the thing + # reported: schema 99 may well have reinstated the field. + assert 'subdir' not in str(excinfo.value) + + +def test_other_root_scalars_stay_a_manifests_own_settings(tmp_path): + """Reserving one root key leaves the root open to a manifest's own settings.""" + setting = f'schema_version = 1\nowner = "proteus"\n{DEMO}' + assert [ds.key for ds in load_manifest(_write(tmp_path, setting))] == ['demo'] From 9b201e3795d765ea9c57a39bfd582fce0ac7b557 Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Sun, 26 Jul 2026 12:46:48 +0200 Subject: [PATCH 2/2] Accept only the schema this fwl-io implements 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. --- docs/Explanations/manifests.md | 6 ++++-- docs/How-to/add_dataset.md | 2 +- src/fwl_io/manifest.py | 39 ++++++++++++++++++++++------------ tests/test_manifest.py | 18 ++++++++++++++++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/docs/Explanations/manifests.md b/docs/Explanations/manifests.md index a50bfc7..1cdc381 100644 --- a/docs/Explanations/manifests.md +++ b/docs/Explanations/manifests.md @@ -25,7 +25,7 @@ Validation at load time: - `extract`, when present, must be `"tar"` or `"zip"`. - A dataset table must not contain sub-tables, and arrays of tables are rejected; ambiguous structures fail loudly instead of being silently dropped. - A dataset table declares only the fields above, and a grouping level declares none: anything else raises `ManifestSchemaError`, naming the field and the manifest schema this fwl-io implements. A model ships its manifest with its own code, so a manifest can be newer than the installed fwl-io; ignoring an unknown field silently would leave the manifest asking for something it never gets, and a `required_by` written one level above its dataset would leave the dataset claiming no model needs it. The message names the action that fits the case: move a dataset field that sits too high, delete a field this fwl-io no longer takes, and for a name it does not know at all, check the spelling or upgrade. Two things are outside the check. A scalar at the manifest root is reserved for a manifest's own settings and is ignored, unless it names a dataset field or `subdir`. And a table is recognised as a dataset by its `zenodo` key, so a misspelt `zenodo` is reported as a table with no pin rather than as an unknown field. -- A manifest may declare the schema it was written against with a root `manifest_schema = `. It is optional, and declaring it sharpens the diagnosis: a number above the schema the installed fwl-io implements can only mean the reader is too old, so the error says to upgrade rather than offering the spelling reading as well. A number the reader does implement rules the upgrade reading out, so an unknown field in that manifest is reported as a misspelling alone. The value must be a whole number of at least 1; `true` is rejected rather than read as 1. +- A manifest may declare the schema it was written against with a root `manifest_schema = `. It is optional, and a manifest that declares one is held to it: only the schema the installed fwl-io implements is accepted. A higher number means the reader is too old, so the error says to upgrade. A lower one means the manifest was written for a schema that stopped loading when the number rose, so the error names both numbers and points here. Accepting only the implemented number is what sharpens the rest: an unknown field in a manifest that declares its schema is reported as a misspelling alone, with no second reading to weigh. The value must be a whole number of at least 1; `true` is rejected rather than read as 1. A table *named* `manifest_schema` is an ordinary directory level, as with `subdir`, and the key written inside a table is reported as misplaced rather than misspelt. - A manifest that fails to load takes its whole provider with it: `discover_manifests` skips that package and logs a warning, so its other datasets disappear from the result too. Use `fwl-io list` to see the error. - A `subdir` field is rejected on any table, dataset or grouping level, and at the manifest root: the location comes from the key. A table *named* `subdir` is an ordinary directory level. - Within one manifest, two keys that differ only in case are rejected: they would share one directory and one registry file on a case-insensitive filesystem. Two installed packages declaring keys that collide is a separate check, tracked in [#18](https://github.com/FormingWorlds/fwl-io/issues/18). @@ -36,7 +36,9 @@ Validation at load time: |---|---|---| | 1 | 26.7.22 | A dataset's location is derived from its dotted table key; `subdir` is not a field. | -An error raised while reading a manifest names the schema the running code implements, so a mismatch between a manifest and an installed fwl-io can be placed against this table. A manifest that declares `manifest_schema` is checked against it directly. +An error raised while reading a manifest names the schema the running code implements, so a mismatch between a manifest and an installed fwl-io can be placed against this table. + +A manifest that declares `manifest_schema` is checked against it directly, and only the implemented number is accepted. Incrementing the schema is therefore a breaking change for any manifest that declares the old one, which is the intent: the number rises precisely when manifests written for the previous one stop loading, so they should fail at the increment with a message naming both numbers rather than part way through a load with a message about some individual field. A manifest that declares nothing is read on a best-effort basis, as before. ## Archive datasets diff --git a/docs/How-to/add_dataset.md b/docs/How-to/add_dataset.md index 02280d7..3e5c29c 100644 --- a/docs/How-to/add_dataset.md +++ b/docs/How-to/add_dataset.md @@ -28,7 +28,7 @@ zenodo = "10.5281/zenodo.1234567" required_by = ["aragog", "zalmoxis", "spider"] ``` -The root `manifest_schema` names the schema the file is written against. It is optional and worth declaring: it lets fwl-io tell a manifest written for a newer schema from a misspelt field, so a load failure names the one that applies instead of offering both. The current schema is in the [schema versions](../Explanations/manifests.md#schema-versions) table. +The root `manifest_schema` names the schema the file is written against. It is optional and worth declaring: it lets fwl-io tell a manifest written for a different schema from a misspelt field, so a load failure names the one that applies instead of offering both. Declaring it also means the manifest has to be updated when the schema number rises, which is the point, since that is when manifests written for the previous number stop loading. The current schema is in the [schema versions](../Explanations/manifests.md#schema-versions) table. The dotted key is the location below `FWL_DATA`, so this dataset lands in `interior/eos/wolf_bower_2018/r`, the version directory named for its Zenodo record. Choose the key to follow the [target layout](../Explanations/manifests.md#the-fwl_data-layout), using only letters, digits, `_` and `-` per segment, each starting with a letter, digit or `_`. `required_by` lists the models whose `fwl-io fetch ` should include this dataset. diff --git a/src/fwl_io/manifest.py b/src/fwl_io/manifest.py index fe64db0..89bf5f2 100644 --- a/src/fwl_io/manifest.py +++ b/src/fwl_io/manifest.py @@ -18,10 +18,12 @@ extract = "tar" # optional: unpack a single-archive deposit The optional root ``manifest_schema`` names the schema the file was written -against. Declaring it sharpens the diagnosis when something does not load: a -number above the schema the installed fwl-io implements can only mean the -reader is too old, and a number equal to it rules that reading out, so an -unknown field is reported as a misspelling alone. +against. A manifest that declares one is held to it: only the schema the +installed fwl-io implements is accepted, a higher number meaning the reader is +too old and a lower one meaning the manifest was written for a schema that +stopped loading when the number rose. That is what sharpens the diagnosis +elsewhere, since an unknown field in such a manifest can only be a +misspelling. The dotted table key is the dataset location below the data root: the table above resolves into ``interior/eos/wolf_bower_2018``. Key segments are @@ -120,12 +122,12 @@ def _unknown_field_error(what: str, declared_schema: int | None = None) -> Manif Without a declared schema both readings are offered, because both are common: a misspelt field, and a manifest written against a schema newer - than the fwl-io reading it. A manifest declaring the schema this code - implements has ruled the second reading out, so the message names the typo - alone. A manifest declaring an older schema does not: the schema number - rises whenever a manifest written for the previous one stops loading, so a - field this code does not know may be one that older schema had and this - one dropped, and both readings stay on the table. + than the fwl-io reading it. A manifest that declares one has ruled the + second reading out by the time this is reached, since the reader admits + only the schema this code implements, so the message names the typo alone. + The equality is restated here rather than assumed, so that relaxing the + reader later cannot silently sharpen the message for a schema it should + not apply to. """ if declared_schema == _MANIFEST_SCHEMA: return ManifestSchemaError( @@ -142,9 +144,12 @@ def _unknown_field_error(what: str, declared_schema: int | None = None) -> Manif def _read_declared_schema(tree: dict) -> int | None: """Return the schema a manifest declares at its root, if it declares one. - Raises when the manifest names a schema this code does not implement, - which is the case the key exists to make unambiguous, and when the value - is not a schema number at all. + Only the schema this code implements is admitted. A higher number means the + reader is too old. A lower one means the manifest is written for a schema + that, by the rule the number follows, stopped loading when it was + incremented; refusing it fails at the increment rather than part way + through a load. Either way the message names both numbers. Raises too when + the value is not a schema number at all. """ if _SCHEMA_KEY not in tree: return None @@ -169,6 +174,14 @@ def _read_declared_schema(tree: dict) -> int | None: f'the manifest declares {_SCHEMA_KEY} {declared}, but this fwl-io reads ' f'{_reading_version()}: upgrade fwl-io to read this manifest.' ) + if declared < _MANIFEST_SCHEMA: + raise ManifestSchemaError( + f'the manifest declares {_SCHEMA_KEY} {declared}, but this fwl-io reads ' + f'manifest schema {_MANIFEST_SCHEMA}: the schema number rises when a ' + f'manifest written for the previous one stops loading, so update the ' + f'manifest against the schema versions table in the manifests ' + f'documentation.' + ) return declared diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 8f89f92..7cf6ddd 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -6,6 +6,7 @@ import pooch import pytest +from fwl_io import manifest from fwl_io.fetch import create_fetcher from fwl_io.manifest import ( Dataset, @@ -877,3 +878,20 @@ def test_other_root_scalars_stay_a_manifests_own_settings(tmp_path): """Reserving one root key leaves the root open to a manifest's own settings.""" setting = f'schema_version = 1\nowner = "proteus"\n{DEMO}' assert [ds.key for ds in load_manifest(_write(tmp_path, setting))] == ['demo'] + + +def test_older_schema_refusal_names_both_numbers(tmp_path, monkeypatch): + """The refusal names the manifest's schema and the reader's, and where to look.""" + monkeypatch.setattr(manifest, '_MANIFEST_SCHEMA', 3) + with pytest.raises(ManifestSchemaError) as excinfo: + load_manifest(_write(tmp_path, f'manifest_schema = 1\n{DEMO}')) + message = str(excinfo.value) + assert 'manifest_schema 1' in message, 'the refusal must name the schema the manifest declares' + assert 'manifest schema 3' in message, 'the refusal must name the schema the reader implements' + assert 'schema versions table' in message, 'the refusal must say where to look' + # It is not the upgrade case: the reader is newer, not older. + assert 'upgrade fwl-io' not in message + + # Discrimination: at the reader's own schema the same file loads, so the + # refusal is the number's doing. + assert load_manifest(_write(tmp_path, f'manifest_schema = 3\n{DEMO}'))[0].key == 'demo'