Skip to content

EC-FR, EC-NL-CROP: override the EuroCrops rows that disagree with the taxonomy - #330

Merged
ivorbosloper merged 7 commits into
mainfrom
ec-nl-fr-supplements
Sep 25, 2026
Merged

ivorbosloper merged 7 commits into
mainfrom
ec-nl-fr-supplements

Conversation

@ivorbosloper

@ivorbosloper ivorbosloper commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes HCAT handling and testing, resolves #304

Depends on fiboa/fiboa.github.io#29

… taxonomy

- ec_nl_crop and ec_fr read the new nl_2020 and fr_2018 supplements
- a source that already carries HCAT (the EuroCrops shapefiles) now gets its
  supplement applied too, keyed on the crop code; so far ec_si's supplement
  was never read

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- test_hcat_codes builds each converter's table with its supplements, so
  KNOWN_BAD can go; fixtures refreshed from fiboa.org where they were stale
- pixi run check-hcat runs the same check against the published tables

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The central HCAT name-correction behavior remains untested, and the changelog inaccurately describes the parsley correction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Applies HCAT supplement corrections to EuroCrops datasets that already contain resolved HCAT fields.

Changes:

  • Adds source-resolved HCAT supplement handling.
  • Configures French, Dutch, and Slovenian EuroCrops supplements.
  • Adds a French fixture and correction test.
File Description
fiboa_cli/​datasets/​commons/​hcat.py Applies supplements to existing HCAT columns.
fiboa_cli/​datasets/​ec_fr.py Configures the French supplement.
fiboa_cli/​datasets/​ec_nl_crop.py Configures the Dutch supplement.
fiboa_cli/​datasets/​ec_si.py Applies supplements using Slovenia’s crop key.
tests/​test_converters.py Tests source-resolved HCAT correction.
tests/​data-files/​convert/​ec_fr/​fr_2018_supplement.csv Adds the French correction fixture.
CHANGELOG.md Documents the corrections.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/test_converters.py Outdated
Comment thread CHANGELOG.md Outdated
ivorbosloper and others added 2 commits September 25, 2026 15:08
Check the HCAT tables as the converters apply them, online too
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ivorbosloper
ivorbosloper marked this pull request as ready for review September 25, 2026 13:16

@m-mohr m-mohr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can judge it, it looks good.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Network failures can crash the new checker, and the EC-NL-CROP override lacks automated coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
Resolved since last review (2)

Comment thread fiboa_cli/datasets/ec_nl_crop.py
Comment thread scripts/check_hcat.py Outdated
Comment thread scripts/check_hcat.py
…each

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The corrections, runtime override behavior, fixtures, and consistency checks are coherent and adequately tested.

Review effort: Balanced
Findings: None

Resolved since last review (3)

@ivorbosloper
ivorbosloper merged commit 814957a into main Sep 25, 2026
8 checks passed
@ivorbosloper
ivorbosloper deleted the ec-nl-fr-supplements branch September 25, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

32 rows across the code lists disagree with the HCAT taxonomy

3 participants