Skip to content

ES-AR: list the per-municipality files from IDEAragon - #263

Merged
ivorbosloper merged 4 commits into
mainfrom
split/es_ar
Sep 18, 2026
Merged

ivorbosloper merged 4 commits into
mainfrom
split/es_ar

Conversation

@ivorbosloper

@ivorbosloper ivorbosloper commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Aragon publishes SIGPAC per municipality rather than as one download, so the converter lists the files from IDEAragon and reads them all.

Aragon publishes SIGPAC per municipality rather than as one download, so the
converter lists them from IDEAragon and reads them all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG

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.

🟡 Changes recommended

The municipality filter prevents runtime discovery, and the new discovery path lacks dedicated test coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates the ES-AR converter to discover and process IDEAragon’s per-municipality SIGPAC archives.

Changes:

  • Adds IDEAragon product discovery and filtering.
  • Updates field mappings and campaign metadata.
  • Adds fixture coverage and changelog documentation.
File summaries
File Summary
tests/test_convert.py Registers ES-AR fixture; discovery-path behavior remains untested.
fiboa_cli/datasets/es_ar.py Adds product discovery and mappings; the municipality filter uses the wrong archive-name position, blocking runtime downloads.
CHANGELOG.md Documents the per-municipality source update.
Review details

Suppressed comments (1)

tests/test_convert.py:87

  • This test supplies input_files, so it exercises only the fixture conversion and bypasses the new get_urls() path. Add a mocked test for the IDEAragon response and assert municipality filtering, generated download URLs, and edition_year; otherwise the central runtime behavior is untested.
    "es_ar": {"variant": "2026", **_input_files("es_ar", "es_ar_44216.shp.zip")},
  • Files reviewed: 3/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

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

Comment thread fiboa_cli/datasets/es_ar.py
Comment on lines +81 to +95
def get_urls(self):
urls = {}
years = set()
for province in PROVINCES:
for product in self.list_products(province):
name = product["name"]
# the intersection also returns neighbouring municipalities
if product["esquema"] != "Municipio" or not name.startswith(province):
continue
urls[DOWNLOAD_URL.format(name=name)] = f"es_ar_{name}.shp.zip"
years.add(str(product["fecha"])[:4])
if not urls:
raise ValueError("No SIGPAC municipality files listed by IDEAragon")
self.edition_year = max(years)
self.info(f"{len(urls)} municipality files, campaign {self.edition_year}")
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ivorbosloper and others added 2 commits September 17, 2026 18:55
…#302)

Reverts the autofix on #263: `recfeg44216_etrs89_h30` is the shapefile inside
the archive, and `44216.shp.zip` is the archive. Filtering on `recfeg` matches
no product, so every run raised "No SIGPAC municipality files".

Adds the test for get_urls that the review asked for; it fails on the
autofixed filter.


Claude-Session: https://claude.ai/code/session_01DVx9uQV2QPM8ecPAY3ZjXG

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ivorbosloper
ivorbosloper merged commit 549dfe5 into main Sep 18, 2026
7 checks passed
@m-mohr
m-mohr deleted the split/es_ar branch September 18, 2026 16:08
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.

3 participants