Skip to content

Add match_station_basin function to identify unique basins for GloFAS stations - #99

Draft
jmargutt wants to merge 6 commits into
mainfrom
station-district-mapping
Draft

Add match_station_basin function to identify unique basins for GloFAS stations#99
jmargutt wants to merge 6 commits into
mainfrom
station-district-mapping

Conversation

@jmargutt

@jmargutt jmargutt commented Apr 8, 2026

Copy link
Copy Markdown
Member

AB#41554

WIP: will be picked up again later.

Describe your changes

Adds match_station_basin() to data/shared/ to map each GloFAS station to the coarsest HydroBASINS level where it is the only station from its country. Intended for use by flood-hazard-logic to determine exposed administrative divisions.

Key implementation details:

  • Iterates HydroBASINS levels 4–12; for each level, spatial-joins stations to basins and counts co-located same-country stations
  • Selects the coarsest (largest) basin where a station is alone (n_same_country == 1) and basin area ≤ max_basin_area_km2
  • Returns a GeoDataFrame keyed by stationCode with basin geometry, HYBAS_ID, and basin_level

CRS safety (addressed in review):

  • Raises ValueError early if stations_gdf.crs is None — prevents silent failure when reprojecting basins to match station CRS
  • Raises ValueError per basin level if a loaded shapefile has no CRS metadata, including the matched filename for easy diagnosis
# stations_gdf must have a CRS set
if stations_crs is None:
    raise ValueError(
        "stations_gdf must have a CRS set. "
        "Assign a CRS with stations_gdf.set_crs(...) before calling this function."
    )
# each basin shapefile must also carry CRS metadata
if gdf.crs is None:
    raise ValueError(
        f"Basin shapefile for level {level} is missing CRS metadata: {filenames}"
    )

Checklist before requesting a code review

  • I have performed a self-review of my code
  • I have addressed all Copilot comments
  • I have asked the design team to review these changes, or: The changes do not touch the UI/UX
  • I have added tests for my changes, or: Adding tests is unnecessary/irrelevant
  • I have made sure that all automated checks pass before requesting a review
  • I have updated all documentation where necessary
  • I do not need any deviation from our PR guidelines

Portal preview-deployment

This PR does not have any preview deployments yet.

Copilot AI review requested due to automatic review settings April 8, 2026 08:02
@jmargutt jmargutt added the enhancement New feature or request that affects our end users label Apr 8, 2026

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.

Pull request overview

Adds a new shared geospatial utility to map each GloFAS station to a “unique” HydroBASINS basin (coarsest level where the station is the only one from that country), intended to support flood hazard exposure/admin-division selection logic.

Changes:

  • Introduces match_station_basin() to compute per-station basin assignment across HydroBASINS levels (4–12).
  • Adds helpers to load basin shapefiles from disk and compute “coarsest unique basin” via spatial joins + per-basin station counts.
  • Returns a GeoDataFrame keyed by stationCode with basin geometry, HYBAS_ID, and selected basin_level.

Comment thread data/shared/match_station_basin.py Outdated
Comment thread data/shared/match_station_basin.py
Comment thread data/shared/match_station_basin.py Outdated
Comment thread data/shared/match_station_basin.py

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

Comment thread data/shared/match_station_basin.py Outdated
Comment thread data/shared/match_station_basin.py Outdated
Comment on lines +83 to +89
# Reproject all basin levels to match the stations CRS so that spatial
# joins and the output geometries are all in a consistent coordinate system
stations_crs = stations_with_country.crs
basins = {
level: gdf.to_crs(stations_crs) if gdf.crs != stations_crs else gdf
for level, gdf in basins.items()
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

If stations_gdf has no CRS set (stations_gdf.crs is None), the basin reprojection step will call GeoDataFrame.to_crs(None) and raise. Add an explicit validation/exception for missing station CRS (or require an explicit CRS parameter) before reprojecting basins.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Done in commit 2647331. Added:

  • An explicit check that raises ValueError when stations_gdf.crs is None, before the reprojection loop.
  • Per-level basin CRS validation inside the reprojection loop: if a loaded basin GeoDataFrame has no CRS, a ValueError is raised with the matching filename(s) to make the failure easy to diagnose.

Comment on lines +155 to +163
for level in range(min_level, max_level + 1):
# Try to find shapefile matching common naming patterns
pattern = os.path.join(basins_dir, f"*lev{level:02d}*.shp")
matches = glob.glob(pattern)
if matches:
print(f" Loading level {level} from {matches[0]} …")
basins[level] = gpd.read_file(matches[0])
else:
print(f" Level {level}: shapefile not found, skipping")

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

_load_basins_from_path() uses glob and silently picks matches[0]. If basins_dir contains multiple HydroBASINS files per level (e.g., multiple regions/continents), this becomes non-deterministic and can select the wrong shapefile. Consider sorting matches and either (a) erroring when len(matches)!=1 or (b) reading all matches and concatenating into one GeoDataFrame for that level.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Comment thread data/shared/match_station_basin.py
Comment thread data/shared/match_station_basin.py
Comment thread data/shared/match_station_basin.py
Comment thread data/shared/match_station_basin.py Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@rodekruis rodekruis deleted a comment from Copilot AI Apr 8, 2026
@rodekruis rodekruis deleted a comment from Copilot AI Apr 8, 2026
@rodekruis rodekruis deleted a comment from Copilot AI Apr 8, 2026
@jmargutt
jmargutt requested review from e3r3i3k3 and p-phung April 8, 2026 09:13
@p-phung p-phung mentioned this pull request Apr 24, 2026
7 tasks
@jannisvisser
jannisvisser marked this pull request as draft May 27, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request that affects our end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants