fix: geometry valid in catchment area, constant as input capacity in 2sfca by CyrineKamoun · Pull Request #3666 · plan4better/goat

CyrineKamoun · 2026-03-11T11:32:30Z

No description provided.

Copilot

Pull request overview

This PR updates GOAT’s shared Python library (goatlib) to improve 2SFCA heatmap configuration (capacity sourcing + docs link/i18n) and to ensure catchment-area outputs are exported with valid polygon geometries.

Changes:

Update the Heatmap 2SFCA tool docs link and add i18n strings for new capacity options.
Extend the 2SFCA opportunity schema + SQL preparation to support capacity from constant / field / polygon expression.
Make exported catchment-area geometries valid (and extract polygons) when writing GeoParquet.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`packages/python/goatlib/src/goatlib/tools/registry.py`	Updates Heatmap 2SFCA `docs_path` to the full doc slug.
`packages/python/goatlib/src/goatlib/i18n/translations/en.json`	Adds new capacity-related translation keys; tweaks demand field description.
`packages/python/goatlib/src/goatlib/i18n/translations/de.json`	Adds new capacity-related translation keys; tweaks demand field description (DE).
`packages/python/goatlib/src/goatlib/analysis/schemas/heatmap.py`	Adds `capacity_type` + related fields to `Opportunity2SFCA` and a validator.
`packages/python/goatlib/src/goatlib/analysis/accessibility/two_step_catchment_area.py`	Implements capacity SQL selection logic and uses it when standardizing opportunities.
`packages/python/goatlib/src/goatlib/analysis/accessibility/catchment_area.py`	Repairs invalid geometries on export and extracts polygon components.

Copilot · 2026-03-11T11:38:40Z

packages/python/goatlib/src/goatlib/analysis/schemas/heatmap.py

+                    "capacity_constant must be set when capacity_type is 'constant'."
+                )
+        elif self.capacity_type == PotentialType.expression:
+            if not self.potential_expression:


validate_capacity_fields checks self.potential_expression in the capacity_type == expression branch, but Opportunity2SFCA defines capacity_expression (and does not define potential_expression). This will raise an AttributeError during validation and also fails to enforce the intended requirement. Update the validator to check self.capacity_expression instead.

Suggested change

if not self.potential_expression:

if not self.capacity_expression:

Copilot · 2026-03-11T11:38:40Z

packages/python/goatlib/src/goatlib/analysis/schemas/heatmap.py

+    capacity_type: PotentialType = Field(
+        default=PotentialType.constant,
+        description="How to determine the capacity value for each opportunity.",
+        json_schema_extra=ui_field(
+            section="opportunities",
+            field_order=4,
+            visible_when={"input_path": {"$ne": None}},
+            widget_options={
+                # Only show "expression" option when input_path is a polygon layer
+                "enum_geometry_filter": {
+                    "source_layer": "input_path",
+                    "expression": ["Polygon", "MultiPolygon"],
+                }
+            },
+        ),
+    )
+
+    capacity_constant: float | None = Field(
+        1.0,
+        gt=0.0,
+        description="Constant capacity value applied to all features.",
+        json_schema_extra=ui_field(
+            section="opportunities",
+            field_order=5,
+            widget="number",
+            visible_when={"input_path": {"$ne": None}, "capacity_type": "constant"},
+        ),
+    )
+
+    capacity_field: str | None = Field(
+        None,
+        description="Field from the opportunity layer that contains the capacity value (e.g., number of beds, seats).",
+        json_schema_extra=ui_field(
+            section="opportunities",
+            field_order=6,
+            label_key="capacity_field",
+            widget="field-selector",
+            widget_options={"source_layer": "input_path", "field_types": ["number"]},
+            visible_when={"input_path": {"$ne": None},  "capacity_type": "field"},
+        ),
+    )
+


capacity_type defaults to constant, but existing callers that only provide capacity_field will now silently get a constant capacity (default 1.0) instead of using the field. This is a breaking behavioral change for 2SFCA. Consider defaulting capacity_type to field (to preserve current behavior) or adding a pre-validation step that infers capacity_type='field' when capacity_field is provided and capacity_type wasn’t explicitly set.

packages/python/goatlib/src/goatlib/analysis/accessibility/two_step_catchment_area.py

Copilot · 2026-03-11T11:38:40Z

packages/python/goatlib/src/goatlib/analysis/accessibility/two_step_catchment_area.py

+        # --- Handle capacity_expression first ---
+        if opp.capacity_expression:
+            expr = opp.capacity_expression.lower().strip()
+
+            if expr in ("$area", "area"):
+                if "polygon" not in geom_type_lower:
+                    raise ValueError(
+                        f"Invalid capacity_expression='{expr}' for geometry type '{geom_type}'. "
+                        "Area is only valid for Polygon or MultiPolygon geometries."
+                    )
+                return f"ST_Area_Spheroid({wgs84_geom_sql})"
+
+            if expr in ("$perimeter", "perimeter"):
+                if "polygon" not in geom_type_lower:
+                    raise ValueError(
+                        f"Invalid capacity_expression='{expr}' for geometry type '{geom_type}'. "
+                        "Perimeter is only valid for Polygon or MultiPolygon geometries."
+                    )
+                return f"ST_Perimeter_Spheroid({wgs84_geom_sql})"
+
+            # Custom user expression (use as-is)
+            return expr
+


_get_capacity_sql lowercases and returns capacity_expression as a raw SQL snippet, but capacity_expression is a PotentialExpression enum (area/perimeter). The “custom user expression” branch is effectively unreachable, and the current precedence also allows capacity_expression to override capacity_type if both are set. Consider switching to logic driven strictly by capacity_type and mapping enum values directly, which will be clearer and prevent surprising overrides.

packages/python/goatlib/src/goatlib/i18n/translations/en.json

Copilot · 2026-03-11T11:38:41Z

packages/python/goatlib/src/goatlib/i18n/translations/en.json

    "demand_field": {
      "label": "Demand Field",
-      "description": "Field with demand values"
+      "description": "Field with population or user number"


English phrasing: "Field with population or user number" is awkward/unclear. Consider rewording to something like "Field with population or number of users" (or keep it generic if demand can be something else).

Suggested change

"description": "Field with population or user number"

"description": "Field with population or number of users"

Copilot · 2026-03-11T11:38:41Z

packages/python/goatlib/src/goatlib/analysis/accessibility/catchment_area.py

            query = f"""
-                SELECT {select_cols}ST_GeomFromText(geometry) AS geometry
+                SELECT {select_cols}ST_CollectionExtract(ST_MakeValid(ST_GeomFromText(geometry)), 3) AS geometry
                FROM gdf_table


This applies ST_MakeValid to every geometry on export, which can be expensive even when geometries are already valid. Consider wrapping it in a CASE WHEN ST_IsValid(...) THEN ... ELSE ST_MakeValid(...) END (and then ST_CollectionExtract) to avoid unnecessary work.

Copilot · 2026-03-11T11:38:41Z

packages/python/goatlib/src/goatlib/analysis/accessibility/catchment_area.py

+                    SELECT {select_cols}ST_CollectionExtract(ST_MakeValid(ST_GeomFromText(geometry)), 3) AS geometry
                    FROM '{temp_path}'


Same as above: ST_MakeValid is run unconditionally during bytes->parquet conversion. Guard with ST_IsValid (or equivalent) so valid geometries don’t pay the full repair cost.

Suggested change

SELECT {select_cols}ST_CollectionExtract(ST_MakeValid(ST_GeomFromText(geometry)), 3) AS geometry

FROM '{temp_path}'

WITH src AS (

SELECT {select_cols}ST_GeomFromText(geometry) AS geom

FROM '{temp_path}'

)

SELECT {select_cols}ST_CollectionExtract(

CASE

WHEN NOT ST_IsValid(geom) THEN ST_MakeValid(geom)

ELSE geom

END,

3

) AS geometry

FROM src

…_step_catchment_area.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

CyrineKamoun added 4 commits March 11, 2026 11:29

fix: constant option for 2sfca capacity

b78bcef

fix: 2sfca unrequired import

a0b0f9a

fix: translation capacity expression

3fa4e29

fix: valid geometry catchment area

12fbeb3

Copilot AI review requested due to automatic review settings March 11, 2026 11:32

Copilot started reviewing on behalf of CyrineKamoun March 11, 2026 11:33 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

CyrineKamoun and others added 3 commits March 11, 2026 11:59

fix: code correction based on pr feedback

d8a7b55

Update packages/python/goatlib/src/goatlib/analysis/accessibility/two…

882545a

…_step_catchment_area.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestion from @Copilot

5cbd844

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

majkshkurti merged commit e74825f into plan4better:main Mar 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: geometry valid in catchment area, constant as input capacity in 2sfca#3666

fix: geometry valid in catchment area, constant as input capacity in 2sfca#3666
majkshkurti merged 7 commits intoplan4better:mainfrom
CyrineKamoun:2.4.0

CyrineKamoun commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if not self.potential_expression:
	if not self.capacity_expression:

	"description": "Field with population or user number"
	"description": "Field with population or number of users"

		SELECT {select_cols}ST_CollectionExtract(ST_MakeValid(ST_GeomFromText(geometry)), 3) AS geometry
		FROM '{temp_path}'

Conversation

CyrineKamoun commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants