LV: editions dated by their campaign, not by the download - #286
Open
ivorbosloper wants to merge 16 commits into
Open
ivorbosloper wants to merge 16 commits into
ivorbosloper wants to merge 16 commits into
Conversation
lv read the current state of the Lauku registrs WFS, so the edition was dated by the day it ran -- 2026 for a snapshot of whatever the register held that morning. Latvia's open data portal publishes the declared areas per campaign, 2015 through 2025, nine regional GeoPackages each, and the files carry the campaign in period_code. The portal's URLs hold dataset and resource UUIDs, so get_urls looks the package up by title: the slug is no guide, reading "klientu-..." up to 2022 and "lauksaimnieku-..." after, and the 2015 package sits under a slug that says 2022. objectid is a row number that restarts at 1 in every regional file -- all 32,186 of Lielriga's also occur in Zemgale -- so the id is the region plus that number, with the region spelled as ASCII because the layer names are not consistent across years. block_number is a real identifier and becomes block_id. The yearly files carry no crop name, which the WFS did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG
The yearly files publish the code without a name, where the WFS carried both. The EuroCrops table already holds the register's name for each code in original_name -- 'Auzas', 'Papuve', 'Ilggadigie zalaji' -- and the mixin reads every column of it except that one. So crop:name is filled from the same table that gives the HCAT columns, and the file reads as it did before: the Latvian name beside the harmonised one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG
EuroCrops' lv_2021.csv holds 138 codes; the 2024 campaign uses 28 more, which is 1.43% of its 424,819 fields and leaves them without a crop name or an HCAT code. Twenty-two of those are variants of a code the table already carries -- "seklas ieguvei" is grown for seed, "ar stiebrzalu paseju" is with grass undersowing, the same plant either way -- so they take that code's HCAT, and lv.csv records which code each was taken from. Four more are named in HCAT directly. 784 and 880 are left unmapped: one field each, and no LAD publication we have names them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG
m-mohr
reviewed
Sep 15, 2026
ivorbosloper
added a commit
that referenced
this pull request
Sep 15, 2026
Carries the converter from split/lv-backfill so the catalog can build the editions; the PR against main holds the same change.
es.py carries the same four lines for the same reason: the base converter splits multi-part geometries only after it has checked the ids.
456 of Saarland's 54,460 application parcels and 1,099 of its 62,981 field blocks are several polygons, and the base converter splits them after it has checked the ids, so the parts shared one. The flik stays the reference it was; only the id gains a part number.
This was referenced Sep 17, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings affect LV variant selection, resource completeness, and multipart metrics.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates the Latvia converter to use campaign-dated annual GeoPackage releases from data.gov.lv, with regional IDs and expanded crop mappings.
Changes:
- Adds annual LV resource discovery for 2015–2025.
- Maps regional/block IDs, crop names, dates, and areas.
- Adds multipart geometry handling and updates related converters, tests, mappings, and changelog.
File summaries
| File | Summary |
|---|---|
tests/test_convert.py |
Updates the Latvia fixture. Nit (1 vote): add coverage for resource discovery, regional completeness, cross-file IDs, and campaign dates. |
tests/data-files/convert/lv/lv.csv |
Expands the Latvia crop-code mapping. |
fiboa_cli/datasets/lv.py |
Implements Latvia resource discovery and field mappings. Moderate (3 votes): initialize the default variant. Moderate (3 votes): validate all nine regional resources. Nit (3 votes): reconcile the documented 28 versus actual 34 added codes. Nit (1 vote): correct the normalization example. |
fiboa_cli/datasets/ec_lt.py |
Formatting-only change. |
fiboa_cli/datasets/de_sl.py |
Updates multipart parcel handling and IDs. |
fiboa_cli/datasets/de_sl_block.py |
Updates multipart block handling and IDs. |
fiboa_cli/conversion/fiboa_converter.py |
Adds shared multipart geometry handling. Moderate (1 vote): recompute per-part metrics after splitting. Nit (1 vote): correct the helper docstring. |
CHANGELOG.md |
Documents the Latvia dataset update. |
Review details
Suppressed comments (4)
fiboa_cli/conversion/fiboa_converter.py:22
- This docstring says the helper generates IDs, but it only validates/explodes geometries and resets the index; each caller generates IDs afterward. That description is misleading for future converters deciding whether this helper provides unique identifiers.
"""
Split multi-part geometries into one row per polygon and generate ids.
Useful for subclasses that generate ids in migrate or post_migrate
"""
fiboa_cli/conversion/fiboa_converter.py:25
explodeduplicates every non-geometry column. In the new LV path this copies a multipart feature's totalshape_areaandshape_lengthonto every polygon;FiboaBaseConverter.post_migrateonly replaces zero areas, so multipart rows report the full feature metrics rather than per-part values. Recompute per-part metrics (or clear them before base recalculation) after splitting.
gdf = gdf.explode(index_parts=False)
gdf = gdf[(gdf.geometry.geom_type == "Polygon") & gdf.geometry.is_valid]
fiboa_cli/datasets/lv.py:112
- The docstring says
Lielrīgaandlielrganormalize to the same value, but this function returnslielrigaandlielrgarespectively. Correct the example or add an explicit alias rule.
"""A region name as ASCII, so Lielrīga and lielrga give the same id."""
tests/test_convert.py:106
- This fixture feeds only one local regional GeoPackage and bypasses
get_urls, so it does not exercise package discovery, nine-region completeness, or the cross-file uniqueness of the new region-prefixed IDs. Add a focused mockedget_urlstest and/or a second-region fixture with assertions for the generated IDs and campaign date.
"bg#2022": {"variant": "2022", **_input_files("bg", "bg_agricultural_land_2022.zip")},
- Files reviewed: 7/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
* EC-FR: the 2018 campaign, from EuroCrops IGN never released an RPG archive for 2018; the downloads run 2017 and 2019 onwards. EuroCrops publishes it with the HCAT columns already resolved, so the collection is theirs rather than a variant of fr. 9,517,878 parcels, ids unique. SURF_PARC is rounded to 0.01 ha, so 4,913 parcels under 50 m2 read as zero and get their area from the geometry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG * EuroCrops: fetch from Zenodo with requests Zenodo answers with two Content-Type headers and the aiohttp client behind the base class rejects that with a 400, so no ec_* converter can download on a cold cache. The failed attempt also left a zero-byte file behind, which the next run would have taken for a cached archive. * EC-FR: split the 13 multi-part parcels before the id is checked * EC-FR: give the parts of a multi-part parcel distinct ids Splitting early only makes that possible; it does not do it. Nine parcels are multi-part, so thirteen rows shared an id. * Update aiohttp, instead of working around it Zenodo answers with two Content-Type headers. aiohttp rejected that until 3.13.5, which skips the RFC 9110 singleton check in lax mode -- the default for responses (aio-libs/aiohttp#12302). The lock held 3.13.3, so no ec_* converter could download on a cold cache. The requests fallback added for it goes again. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Matthias Mohr <m.mohr@moregeo.it>
…r-part metrics vecorel-cli 0.2.20 chooses the first declared variant in convert(), before get_urls() runs, so a converter that overrides get_urls() no longer has to repeat the default. Eleven converters drop their copy, LV gains the default it never had (it raised without --variant), and ES-PV's default becomes its newest variant like everywhere else. ES sets the determination from the variant in post_migrate instead of __init__, where the variant is not yet known. publish selects the variant the same way, so the README lists the source files of the published edition rather than of the newest one. LV requires the nine regional GeoPackages of a campaign instead of any two, and the code list comment counts the 34 codes it actually adds. split_multipart() marks the rows it splits and post_migrate() recomputes their area and perimeter, which explode() had copied from the whole feature.
# Conflicts: # fiboa_cli/conversion/fiboa_converter.py
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Package selection can match unrelated datasets, and metric handling introduces unnecessary full-dataset reprojections.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 24/28 changed files
- Comments generated: 3
- Review effort level: Balanced
Contributor
|
This requires vecorel/cli#45 now |
…V changelog entry post_migrate reprojected every dataset in a geographic CRS to EPSG:6933, even where no area was computed; now only the geometries whose area or perimeter is calculated are reprojected. LV's package lookup requires the dataset phrase in the title, since the CKAN search also matches descriptions. The merge had kept the LV changelog line twice. The default variant is the latest year (vecorel-cli 0.2.20), which moves FR's default from 2022 to 2024; the FR fixture is pinned to 2022.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Multipart handling changes Saarland’s documented area semantics, and unconditional CRS access introduces a regression for CRS-less data.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 24/28 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+93
to
+95
| # a block can be several polygons, and the base converter splits them only after it | ||
| # has checked the ids, so the parts would share one | ||
| gdf = self.split_multipart(gdf) |
| # If CRS is not in meters, reproject to an equal-area projection for area calculation | ||
| crs_is_in_meters = gdf.crs.axis_info[0].unit_name in ("m", "metre", "meter") | ||
| area_key = self._source_column(AREA_KEY) | ||
| crs_is_in_meters = gdf.crs.axis_info[0].unit_name in ("m", "metre", "meter") |
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.
lvread the current state of the Lauku reģistrs WFS, so its edition was dated by the day the conversion ran.This reads the yearly releases on data.gov.lv instead: editions 2015-2025, nine regional GeoPackages each, dated by the campaign they hold.
get_urlslooks the package up by title; its URLs carry UUIDs and the slug changes across years.objectidrestarts at 1 in every regional file, so the id is the region plus that number.block_numberbecomesblock_id.crop:namecomes from the code list, which the files do not carry.