[BUG] Divisions: allow both is_land and is_territorial to be true

examples/divisions/division_area/both_land_territorial.yaml

@@ -0,0 +1,27 @@
+---
+id: example:division_area:both_land_territorial:country:us
+type: Feature
+geometry:
+  type: Polygon
+  coordinates: [
+    [
+      [-82.8732511, 24.4116731],
+      [-82.5948517, 24.5902399],
+      [-82.7300073, 24.8395704],
+      [-83.153058, 24.6776636],
+      [-82.8732511, 24.4116731]
+    ]
+  ]
+properties:
+  theme: divisions
+  type: division_area
+  subtype: country
+  is_land: true
+  is_territorial: true
+  country: US
+  admin_level: 0
+  version: 0
+  class: land
+  division_id: example:division:country:us
+  names:
+    primary: United States

reference/counterexamples/divisions/division_boundary/bad-not-both.yaml → examples/divisions/division_boundary/both_land_territorial.yaml

@@ -1,5 +1,5 @@
 ---
-id: example:division_boundary:is_land:country:us
+id: example:division_boundary:both_land_territorial:country:us
 type: Feature
 geometry:
   type: LineString
@@ -9,9 +9,8 @@ properties:
   type: division_boundary
   version: 1
   subtype: country
+  admin_level: 0
   is_land: true
   is_territorial: true
   class: land
   division_ids: ["example:division:country:left", "example:division:country:right"]
-  ext_expected_errors:
-    - "oneOf failed, subschemas 0, 1 matched"

packages/overture-schema-codegen/src/overture/schema/codegen/extraction/model_constraints.py

@@ -18,6 +18,7 @@
     Not,
     RadioGroupConstraint,
     RequireAnyOfConstraint,
+    RequireAnyTrueConstraint,
     RequireIfConstraint,
 )
 
@@ -71,6 +72,12 @@ def _describe_condition(condition: object) -> str:
     return str(condition)
 
 
+def _unwrap_true_field_eq(condition: object) -> FieldEqCondition | None:
+    if isinstance(condition, FieldEqCondition) and condition.value is True:
+        return condition
+    return None
+
+
 def _describe_conditional(constraint: _ConditionalConstraint) -> str:
     """Describe a require_if or forbid_if constraint."""
     fields = _backtick_join(constraint.field_names)
@@ -143,6 +150,10 @@ def _affected_field_names(constraint: ModelConstraint) -> frozenset[str]:
         )
     if isinstance(constraint, (RequireAnyOfConstraint, RadioGroupConstraint)):
         return frozenset(constraint.field_names)
+    if isinstance(constraint, RequireAnyTrueConstraint):
+        return frozenset().union(
+            *(_condition_field_names(condition) for condition in constraint.conditions)
+        )
     return frozenset()
 
 
@@ -152,6 +163,21 @@ def _describe_one(constraint: ModelConstraint) -> str | None:
         return None
     if isinstance(constraint, RequireAnyOfConstraint):
         return f"At least one of {_backtick_join(constraint.field_names)} must be set"
+    if isinstance(constraint, RequireAnyTrueConstraint):
+        true_field_conditions = [
+            field_eq
+            for condition in constraint.conditions
+            if (field_eq := _unwrap_true_field_eq(condition)) is not None
+        ]
+        if len(true_field_conditions) == len(constraint.conditions):
+            return (
+                "At least one of "
+                f"{_backtick_join(tuple(c.field_name for c in true_field_conditions))} "
+                "must be `true`"
+            )
+        return "At least one of these conditions must be true: " + ", ".join(
+            _describe_condition(condition) for condition in constraint.conditions
+        )
     if isinstance(constraint, RadioGroupConstraint):
         return f"Exactly one of {_backtick_join(constraint.field_names)} must be `true`"
     if isinstance(constraint, MinFieldsSetConstraint):

packages/overture-schema-codegen/tests/test_constraint_description.py

@@ -19,6 +19,7 @@
     Not,
     RadioGroupConstraint,
     RequireAnyOfConstraint,
+    RequireAnyTrueConstraint,
     RequireIfConstraint,
 )
 from overture.schema.system.primitive import GeometryType, GeometryTypeConstraint
@@ -59,6 +60,28 @@ def test_radio_group(self) -> None:
 
         assert result == ["Exactly one of `is_land`, `is_territorial` must be `true`"]
 
+    def test_require_any_true_true_fields(self) -> None:
+        constraint = RequireAnyTrueConstraint._create_internal(
+            "@require_any_true",
+            FieldEqCondition("is_land", True),
+            FieldEqCondition("is_territorial", True),
+        )
+        result = describe_model_constraints((constraint,))
+
+        assert result == ["At least one of `is_land`, `is_territorial` must be `true`"]
+
+    def test_require_any_true_generic_conditions(self) -> None:
+        constraint = RequireAnyTrueConstraint._create_internal(
+            "@require_any_true",
+            FieldEqCondition("field_a", "foo"),
+            FieldEqCondition("field_b", "bar"),
+        )
+        result = describe_model_constraints((constraint,))
+
+        assert result == [
+            "At least one of these conditions must be true: `field_a` = `foo`, `field_b` = `bar`"
+        ]
+
     def test_min_fields_set(self) -> None:
         constraint = MinFieldsSetConstraint._create_internal("@min_fields_set", 3)
         result = describe_model_constraints((constraint,))
@@ -218,7 +241,11 @@ def test_division_like_model(self) -> None:
                 ["level"],
                 FieldEqCondition("subtype", "region"),
             ),
-            RadioGroupConstraint._create_internal("@radio_group", "is_land", "is_sea"),
+            RequireAnyTrueConstraint._create_internal(
+                "@require_any_true",
+                FieldEqCondition("is_land", True),
+                FieldEqCondition("is_sea", True),
+            ),
         )
         result = describe_model_constraints(constraints)
 
@@ -227,7 +254,7 @@ def test_division_like_model(self) -> None:
             "`parent_id` is forbidden when `subtype` = `country`",
             "`parent_id` is required when `subtype` ≠ `country`",
             "`level` is required when `subtype` is one of: `country`, `region`",
-            "Exactly one of `is_land`, `is_sea` must be `true`",
+            "At least one of `is_land`, `is_sea` must be `true`",
         ]
 
 
@@ -311,6 +338,17 @@ def test_radio_group_maps_all_fields(self) -> None:
         expected = "Exactly one of `is_land`, `is_sea` must be `true`"
         assert result == {"is_land": [expected], "is_sea": [expected]}
 
+    def test_require_any_true_maps_condition_fields(self) -> None:
+        constraint = RequireAnyTrueConstraint._create_internal(
+            "@require_any_true",
+            FieldEqCondition("is_land", True),
+            FieldEqCondition("is_sea", True),
+        )
+        result = field_constraint_notes((constraint,))
+
+        expected = "At least one of `is_land`, `is_sea` must be `true`"
+        assert result == {"is_land": [expected], "is_sea": [expected]}
+
     def test_multiple_constraints_on_one_field(self) -> None:
         """Field appearing in multiple constraints gets all descriptions."""
         c1 = RequireAnyOfConstraint._create_internal(

packages/overture-schema-codegen/tests/test_integration_real_models.py

@@ -234,28 +234,27 @@ def test_segment_common_base_is_base_model(self, segment_spec: UnionSpec) -> Non
         assert "id" in segment_spec.common_base.model_fields
 
 
-class TestPydanticTypePages:
-    """End-to-end: pipeline produces pages for referenced Pydantic built-in types."""
+@pytest.fixture(scope="module")
+def pages() -> list:
+    """Generate all pages from real discovered models."""
+    models = discover_models()
+    feature_specs: list[FeatureSpec] = []
+    for key, entry in models.items():
+        if is_model_class(entry):
+            feature_specs.append(extract_model(entry, entry_point=key.entry_point))
+        elif is_union_alias(entry):
+            feature_specs.append(
+                extract_union(
+                    entry_point_class(key.entry_point),
+                    entry,
+                    entry_point=key.entry_point,
+                )
+            )
+    return generate_markdown_pages(feature_specs, "overture.schema")
 
-    _SCHEMA_ROOT = "overture.schema"
 
-    @pytest.fixture(scope="class")
-    def pages(self) -> list:
-        """Generate all pages from real discovered models."""
-        models = discover_models()
-        feature_specs: list[FeatureSpec] = []
-        for key, entry in models.items():
-            if is_model_class(entry):
-                feature_specs.append(extract_model(entry, entry_point=key.entry_point))
-            elif is_union_alias(entry):
-                feature_specs.append(
-                    extract_union(
-                        entry_point_class(key.entry_point),
-                        entry,
-                        entry_point=key.entry_point,
-                    )
-                )
-        return generate_markdown_pages(feature_specs, self._SCHEMA_ROOT)
+class TestPydanticTypePages:
+    """End-to-end: pipeline produces pages for referenced Pydantic built-in types."""
 
     def test_http_url_page_exists(self, pages: list) -> None:
         """Pipeline produces a page for HttpUrl under pydantic/networks/."""

packages/overture-schema-common/tests/scoping/test_scoped.py

@@ -142,7 +142,8 @@ class Qux(BaseModel):
                 pass
 
     @pytest.mark.parametrize(
-        "scope,required", itertools.product(Scope._top_level_scopes(), (False, True))
+        "scope,required",
+        list(itertools.product(Scope._top_level_scopes(), (False, True))),
     )
     def test_single_scope_top_level(self, scope: Scope, required: bool) -> None:
         if required:
@@ -162,7 +163,8 @@ class SingleScoped(BaseModel):
         assert field_info.is_required() == required
 
     @pytest.mark.parametrize(
-        "scope,required", itertools.product(Scope._when_scopes(), (False, True))
+        "scope,required",
+        list(itertools.product(Scope._when_scopes(), (False, True))),
     )
     def test_single_scope_when(self, scope: Scope, required: bool) -> None:
         if required:

packages/overture-schema-divisions-theme/src/overture/schema/divisions/division_area.py

@@ -15,7 +15,7 @@
 from overture.schema.system.doc import DocumentedEnum
 from overture.schema.system.model_constraint import (
     FieldEqCondition,
-    radio_group,
+    require_any_true,
     require_if,
 )
 from overture.schema.system.primitive import (
@@ -53,7 +53,10 @@ class AreaClass(str, DocumentedEnum):
 @require_if(["admin_level"], FieldEqCondition("subtype", DivisionSubtype.REGION))
 @require_if(["admin_level"], FieldEqCondition("subtype", DivisionSubtype.MACROCOUNTY))
 @require_if(["admin_level"], FieldEqCondition("subtype", DivisionSubtype.COUNTY))
-@radio_group("is_land", "is_territorial")
+@require_any_true(
+    FieldEqCondition("is_land", True),
+    FieldEqCondition("is_territorial", True),
+)
 class DivisionArea(
     OvertureFeature[Literal["divisions"], Literal["division_area"]], Named
 ):

packages/overture-schema-divisions-theme/src/overture/schema/divisions/division_boundary.py

@@ -14,7 +14,7 @@
 from overture.schema.system.model_constraint import (
     FieldEqCondition,
     forbid_if,
-    radio_group,
+    require_any_true,
     require_if,
 )
 from overture.schema.system.primitive import (
@@ -57,7 +57,10 @@ class BoundaryClass(str, DocumentedEnum):
 @require_if(["admin_level"], FieldEqCondition("subtype", DivisionSubtype.REGION))
 @require_if(["admin_level"], FieldEqCondition("subtype", DivisionSubtype.MACROCOUNTY))
 @require_if(["admin_level"], FieldEqCondition("subtype", DivisionSubtype.COUNTY))
-@radio_group("is_land", "is_territorial")
+@require_any_true(
+    FieldEqCondition("is_land", True),
+    FieldEqCondition("is_territorial", True),
+)
 class DivisionBoundary(
     OvertureFeature[Literal["divisions"], Literal["division_boundary"]]
 ):

packages/overture-schema-divisions-theme/tests/division_area_baseline_schema.json

@@ -489,29 +489,35 @@
           }
         }
       ],
-      "not": {
-        "required": [
-          "id",
-          "bbox",
-          "geometry"
-        ]
-      },
-      "oneOf": [
+      "anyOf": [
         {
           "properties": {
             "is_land": {
               "const": true
             }
-          }
+          },
+          "required": [
+            "is_land"
+          ]
         },
         {
           "properties": {
             "is_territorial": {
               "const": true
             }
-          }
+          },
+          "required": [
+            "is_territorial"
+          ]
         }
       ],
+      "not": {
+        "required": [
+          "id",
+          "bbox",
+          "geometry"
+        ]
+      },
       "patternProperties": {
         "^ext_.*$": {
           "description": "Additional top-level properties are allowed if prefixed by `ext_`.\n\nThis feature is a on a deprecation path and will be removed once the schema is\nfully migrated to Pydantic."

packages/overture-schema-divisions-theme/tests/division_boundary_baseline_schema.json

@@ -417,29 +417,35 @@
           }
         }
       ],
-      "not": {
-        "required": [
-          "id",
-          "bbox",
-          "geometry"
-        ]
-      },
-      "oneOf": [
+      "anyOf": [
         {
           "properties": {
             "is_land": {
               "const": true
             }
-          }
+          },
+          "required": [
+            "is_land"
+          ]
         },
         {
           "properties": {
             "is_territorial": {
               "const": true
             }
-          }
+          },
+          "required": [
+            "is_territorial"
+          ]
         }
       ],
+      "not": {
+        "required": [
+          "id",
+          "bbox",
+          "geometry"
+        ]
+      },
       "patternProperties": {
         "^ext_.*$": {
           "description": "Additional top-level properties are allowed if prefixed by `ext_`.\n\nThis feature is a on a deprecation path and will be removed once the schema is\nfully migrated to Pydantic."

packages/overture-schema-system/README.md

@@ -88,6 +88,7 @@ class Contact(BaseModel):
 ```
 
 - `@require_any_of("a", "b", ...)` -- at least one field must be non-None
+- `@require_any_true(cond1, cond2, ...)` -- at least one condition must evaluate to true
 - `@radio_group("a", "b", ...)` -- at most one field may be truthy
 - `@require_if("target", condition)` -- field required when condition holds
 - `@forbid_if("target", condition)` -- field forbidden when condition holds