FiboaBaseConverter: make id mean something - #265
Open
ivorbosloper wants to merge 1 commit into
Open
Conversation
This was referenced Sep 11, 2026
ivorbosloper
force-pushed
the
split/base-drops
branch
from
September 12, 2026 12:49
aa580de to
8da77df
Compare
ivorbosloper
force-pushed
the
split/base-identity
branch
2 times, most recently
from
September 12, 2026 12:55
a2e433c to
d47e82a
Compare
ivorbosloper
force-pushed
the
split/base-drops
branch
from
September 12, 2026 15:10
8da77df to
5270f8a
Compare
ivorbosloper
force-pushed
the
split/base-identity
branch
from
September 12, 2026 15:13
d47e82a to
7978a4a
Compare
This was referenced Sep 12, 2026
ivorbosloper
changed the title
id mean something, and refuse a silent variant trapid mean something
…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
ivorbosloper
force-pushed
the
split/base-drops
branch
from
September 12, 2026 16:13
5270f8a to
2127c77
Compare
ivorbosloper
force-pushed
the
split/base-identity
branch
from
September 12, 2026 16:13
7978a4a to
c0ba3e5
Compare
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
1 participant
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.
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