PT: add the 2017 to 2022 editions - #228
Open
haithcoatj wants to merge 4 commits into
Open
haithcoatj wants to merge 4 commits into
haithcoatj wants to merge 4 commits into
Conversation
m-mohr
requested changes
Sep 9, 2026
ivorbosloper
pushed a commit
to fieldsoftheworld/harmonized-field-data-catalog
that referenced
this pull request
Sep 10, 2026
The three parquet are already in the bucket; the file:size and file:checksum here were measured against the published objects. 2023 and 2025 are unchanged. Their metadata was regenerated from the published bytes pulled back into staging, verified by multihash against the committed checksums, and came out byte-identical, so they carry no diff. Depends on fiboa/cli#228 for the converter.
Contributor
|
@haithcoatj We discussed this today in a meeting with Ivor, Abass and me. We will extract the converter chanages from #214 into separate smaller PRs that can be reviewed more easily, and then close #214. So opening PRs that merge into #214 will eventually also close. Thus, this PR should extract all relevant changes for the PT converter into this or a separate PR so that it can be reviewed separately and merged into main. |
ivorbosloper
pushed a commit
that referenced
this pull request
Sep 12, 2026
publish-portolan still carries the pre-#227 de_sh.py, so anyone rebuilding de_sh from this branch reproduces all three bugs #223/#227 fixed: the 2023 archive matches no GDAL driver, fachguelti reaches the STAC step as DD.MM.YYYY text, and the 2025/2026 upper-case spellings silently drop determination:datetime and metrics:area while still validating. The 2026-shaped test fixture comes with it. #228 brings the expected_columns delivery assertion to this branch, and an assertion whose fixture is still 2024-shaped cannot bite -- 2024 already spells the columns the way the converter maps them, so nothing exercises COLUMN_RENAMES or the decimal-comma area. That combination is exactly what hid the bug in the first place. main has had both since 2026-09-08 but has not been merged into publish-portolan; afterwards both files are identical on the two branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
post_migrate writes determination:datetime but nothing adds it to columns, so the
base converter's "remove all columns that are not listed" step deletes it again.
The CHANGELOG on this branch already claims this fix ("the determination:datetime
column was dropped again because it was not listed in columns") but the code
doesn't have it — it only exists on publish-portolan.
Hits DK and HR too, not just PT. Without it all five pt test entries fail with
pt dropped ['determination:datetime'].
haithcoatj
force-pushed
the
pt-2020-2022
branch
from
September 14, 2026 21:39
1563b7c to
6aabae6
Compare
Contributor
Author
These editions publish the sub-parcel geometry as eight or nine regional files in four projections, so each is reprojected to WGS 84 before they're merged. The crop code is C1 on the layer in 2022 and joined in from a separate table in 2020 and 2021 (left join, validate="many_to_one", row counts asserted per region; max multiplicity 1, so no fan-out). Culturas_2021.dbf has fourteen rows keyed Osa_id = 0 with every crop column NULL and no matching geometry anywhere in the edition, which makes the key non-unique — they're dropped, and no output row changes. The layer filter is per edition rather than the shared DATA_LAYER pattern, because Culturas_.+ matches Culturas_2021, an attribute-only table with no geometry. None of these editions publishes CUL_ID. IFAP recorded one row per land occupation then, with crops as attributes or in a joined table, so there's no crop parcel to identify — the occupation identifies the field and its parcel is the block. 2020's crop table stops at the mainland, so its 199,123 island fields carry no crop code. 2021 and 2022 are unaffected. Also pins pt to 2023 and pt#2025 to 2025 in extra_convert_parameters. With no entries both ran the first variant key and the 2023 Continente.gpkg fixture wasn't exercising anything.
These publish the crop as a Portuguese name rather than a code, so the name is resolved against pt.csv original_name before AddHCATMixin runs, since the mixin keys on original_code whenever the mapping CSV has that column. The name itself is kept as crop:name; these and 2023 are the only editions that publish one. 2018 ships the north twice. Ocupacoes_solo_Norte_S repeats 154,980 of Ocupacoes_solo_Norte_N's fields exactly, same PAR_NUM, same crop, geometry equal to 1e-9. Norte_S is not wholly contained in Norte_N, 386,611 of its rows are unique, so the member cannot be dropped and the collision is resolved row by row. Overlap between any other pair of members raises, because every other pair in every edition is disjoint. 2017's two island files publish no OSA_ID at all and repeat PAR_NUM, so their 177,614 fields get ids from their position under a stable sort on (PAR_NUM, representative point), based far above the largest OSA_ID ever published. They are kept rather than dropped: 2018's Azores file is 140,532 comparable rows that carry an identifier, and the difference is the provider's bookkeeping, not the data. Fourteen 2018 crop names lost an accent to a literal "?". Each was resolved once, offline, to exactly one pt.csv entry and hard-coded; an unresolved "?" raises rather than going null, because it cannot be recovered at runtime. Crop columns are found by number, not by name: the count varies per file rather than per year (2017 to c12, 2019's Azores file to C28), the case is mixed inside one year, and the range can skip a value, so 2018's Norte_N1 has c1..c7 and c9. layer_filter accepts everything for these editions because they are shapefiles whose single layer is named after the region; MEMBERS is what selects. Every source area column is ignored. Three of the thirty files publish one and they disagree on units, square metres in 2017's islands and hectares in 2018's Norte_N1, so area is measured from geometry for all of them. The uniqueness assertion deliberately runs in post_migrate, before the base converter explodes multipart geometries: it guarantees that no two source rows claim the same land occupation, not that ids are unique in the output, which they are not once a multipart field becomes several rows.
2017's two island files publish no OSA_ID, so ids are synthesised from ISLAND_ID_BASE. That range is only safe because no land occupation IFAP numbers comes near it: the largest in any edition from 2017 to 2025 is 46,449,461. Nothing enforced that, so lowering the base or reusing this path for a source with larger identifiers would have silently merged two id namespaces.
haithcoatj
force-pushed
the
pt-2020-2022
branch
from
September 18, 2026 21:14
6aabae6 to
32e8028
Compare
haithcoatj
changed the title
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
2 participants
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.
Adds pt 2017 through 2022, retargeted onto #243 rather than #214.
#243 currently conflicts with main so this can't merge until that's rebased, and I force-pushed to fold two comment fixes into their parent commits, so the commit list changed.
7533e9b is a four-line base fix taken from publish-portolan. post_migrate writes determination:datetime but nothing lists it in columns, so it gets stripped again. Without it every pt test fails. Separate commit, can split it out.
Also, I added extra_convert_parameters entries pinning pt to 2023 and pt#2025 to 2025; without them both ran the first variant key and the 2023 fixture exercised nothing. And ~20% of 2017-2019 rows have no crop, shipped as an empty string rather than NULL, because crop:code is required in the crop extension so a null cannot be written at all. That is harmonized-field-data-catalog#21 at larger scale.
I can split 2017-2019 into its own PR if that is preferred.