FiboaBaseConverter: make id mean something - #265
ivorbosloper wants to merge 1 commit into
Conversation
aa580de to
8da77df
Compare
a2e433c to
d47e82a
Compare
8da77df to
5270f8a
Compare
d47e82a to
7978a4a
Compare
id mean something, and refuse a silent variant trapid mean something
5270f8a to
2127c77
Compare
c0ba3e5 to
f478bfd
Compare
008f70e to
b5a5921
Compare
…iant trap
Four rules, each of which was broken somewhere in the catalog without anything
noticing.
**A converter must map something to `id`.** Nothing downstream enforced it: the
base converter drops columns no mapping names, so a converter without one wrote
a file with no identifier and it validated. That is how de_bb and de_nds reached
the catalog. sk shows the subtler half — `index_as_id = True` fills the column
and the same drop step removes it again, because `columns` never named it.
**The column must be in the source, not only in the mapping.** si's 2019
campaign names the field POLJINA_ID where every later one names it ID, so the
drop step wrote 820,151 fields with no id — and that validated too.
**The values must be unique.** An audit found 36 published collections whose
latest edition repeats an id, and the catalog documents `id` as unique within an
edition. es_cl gave 9,109,136 fields the id "0"; us_usda_cropland gave 3,093
distinct ids to 7.5 million fields. The check runs before geometries are
exploded, so it judges what the converter assigned rather than the parts of a
split feature, and it ignores rows with no id at all, which the bounded drop
removes a few lines later.
**A converter may not declare both `sources` and `variants`.** `sources` wins in
the base converter and the variants are ignored entirely, so `--variant 2011`
silently converts whatever `sources` points at: hr would have published thirteen
copies of the current file as thirteen years. A converter that inherits variants
it does not want says so with `variants = {}`, as ec_be_vlg now does.
Three sweep tests run all four rules over every converter in the repository.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG
f478bfd to
9755e4f
Compare
|
vecorel-cli 0.2.18 carries this upstream. Its #280 bumps the pin and brings along the two converter fixes the checks fire on (hr's rolling |
|
We should still discuss whether we apply a semantic to the id field. Some have stable IDs now, some are just row indeices. |
Based on #264, not on main, because both touch the same file — review that one first. Four rules, each broken somewhere in the published catalog without anything noticing.
idskis the subtler half:index_as_idfills the column and the same drop removes it, becausecolumnsnever named itPOLJINA_IDwhere later campaigns name itID; 820,151 fields were written with no id, and that validated too"0", us_usda_cropland 3,093 ids to 7.5M fieldssourcesandvariantssourceswins, so--variant 2011converts whateversourcespoints at — hr would have published thirteen copies of one fileThe uniqueness check runs before geometries are exploded and ignores rows with no id (those are #264's bounded drop), and it reads tuple targets, because de_sh maps
FLIKto bothflikandid.CI stays red until #246, #244 and #241 land — the sweeps run over every converter, and those three still fail on main. That ordering is the point.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG