Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
@sjpfenninger ready for review, I assume? |
* Allow per-subunit tech configuration override * Clean up download rules, download full TIF files by default * Improve plotting
There was a problem hiding this comment.
@sjpfenninger Looks good overall!
A few comments. You can choose to ignore nit-picks or optional stuff.
Most important:
- the need to update to the newest template version before merging this in (changes should be minor in your case).
- a few clarifications regarding the configuration schema
|
The new ´override´ key in the config is currently not optional. If it's missing, rule area_potential throws a key error. |
|
@sjpfenninger wile digging around in the integrated module workflow, I found a way to avoid having to force users to download the WDPA dataset on their own. the following # Full world polygons to GeoJSON
curl -G "https://data-gis.unep-wcmc.org/server/rest/services/ProtectedSites/The_World_Database_of_Protected_Areas/FeatureServer/1/query" \
--data-urlencode "where=1=1" \ # ask for all records
--data-urlencode "outFields=*" \ # ask for all columns
--data-urlencode "outSR=4326" \ # WGS84 (can be another EPSG)
--data-urlencode "f=geojson" \ # GeoJSON, convert to GeoParquet later
-o wdpa_poly_latest.geojsonThis results in a ~180 MB download that is completed rather quickly Another alternative is the # Polygons (layer 1) -> GeoParquet
ogr2ogr -f Parquet wdpa_poly_latest.parquet \
"https://data-gis.unep-wcmc.org/server/rest/services/ProtectedSites/The_World_Database_of_Protected_Areas/FeatureServer/1" \
-t_srs EPSG:4326 \
-lco COMPRESSION=SNAPPY -lco GEOMETRY_ENCODING=WKB
|
|
@irm-codebase Could you check if you are satisfied with the changes made in response to your review? And thanks for the WDPA data hint - I've moved that to an issue to revisit later, but the API seems hard-limited to 2000 records, so it may not be feasible to use. |
irm-codebase
left a comment
irm-codebase
left a comment
There was a problem hiding this comment.
Looks good overall!
It seems like the integration test is failing, though.
consider this approved, but the integration test must pass.
workflow/scripts/_schemas.py
Outdated
| geometry: GeoSeries[Point] = Field() | ||
| "Shape polygon." |
There was a problem hiding this comment.
Why Point? This one will likely fail... shapefiles can be polygons or multipolygons. I would not validate that.
There was a problem hiding this comment.
Good question - I've now used a subset of the schema from module_geo_boundaries 1:1, to ensure that it is consistent.
4 participants
Initial implementation
Missing:
DocsLoggingReviewer checklist
INTERFACE.yamlis up-to-date with all relevant user resources and results.