[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,
 )
 
@@ -141,7 +142,10 @@ def _affected_field_names(constraint: ModelConstraint) -> frozenset[str]:
         return frozenset(constraint.field_names) | _condition_field_names(
             constraint.condition
         )
-    if isinstance(constraint, (RequireAnyOfConstraint, RadioGroupConstraint)):
+    if isinstance(
+        constraint,
+        (RequireAnyOfConstraint, RadioGroupConstraint, RequireAnyTrueConstraint),
+    ):
         return frozenset(constraint.field_names)
     return frozenset()
 
@@ -152,6 +156,10 @@ 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):
+        return (
+            f"At least one of {_backtick_join(constraint.field_names)} must be `true`"
+        )
     if isinstance(constraint, RadioGroupConstraint):
         return f"Exactly one of {_backtick_join(constraint.field_names)} must be `true`"
     if isinstance(constraint, MinFieldsSetConstraint):

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,7 @@ 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("is_land", "is_territorial")
 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,7 @@ 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("is_land", "is_territorial")
 class DivisionBoundary(
     OvertureFeature[Literal["divisions"], Literal["division_boundary"]]
 ):

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

@@ -468,14 +468,7 @@
           }
         }
       ],
-      "not": {
-        "required": [
-          "id",
-          "bbox",
-          "geometry"
-        ]
-      },
-      "oneOf": [
+      "anyOf": [
         {
           "properties": {
             "is_land": {
@@ -491,6 +484,13 @@
           }
         }
       ],
+      "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

@@ -396,14 +396,7 @@
           }
         }
       ],
-      "not": {
-        "required": [
-          "id",
-          "bbox",
-          "geometry"
-        ]
-      },
-      "oneOf": [
+      "anyOf": [
         {
           "properties": {
             "is_land": {
@@ -419,6 +412,13 @@
           }
         }
       ],
+      "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/src/overture/schema/system/model_constraint/__init__.py

@@ -12,6 +12,7 @@
 from .no_extra_fields import NoExtraFieldsConstraint, no_extra_fields
 from .radio_group import RadioGroupConstraint, radio_group
 from .require_any_of import RequireAnyOfConstraint, require_any_of
+from .require_any_true import RequireAnyTrueConstraint, require_any_true
 from .require_if import RequireIfConstraint, require_if
 
 __all__ = [
@@ -31,7 +32,9 @@
     "radio_group",
     "RadioGroupConstraint",
     "require_any_of",
+    "require_any_true",
     "require_if",
     "RequireAnyOfConstraint",
+    "RequireAnyTrueConstraint",
     "RequireIfConstraint",
 ]

packages/overture-schema-system/src/overture/schema/system/model_constraint/require_any_true.py

@@ -0,0 +1,152 @@
+"""
+Require at least one field in a group of `bool` fields to have the value `True`.
+"""
+
+from collections.abc import Callable
+from types import NoneType, UnionType
+from typing import Annotated, Any, Union, get_args, get_origin
+
+from pydantic import BaseModel, ConfigDict
+from typing_extensions import override
+
+from .._json_schema import get_static_json_schema_extra, put_any_of
+from .model_constraint import FieldGroupConstraint, apply_alias
+
+
+def require_any_true(*field_names: str) -> Callable[[type[BaseModel]], type[BaseModel]]:
+    """
+    Decorate a Pydantic model class with a constraint requiring that at least one field in a group
+    of `bool` fields has the value `True`.
+
+    This function is the decorator version of the `RequireAnyTrueConstraint` class.
+
+    Unlike `radio_group`, which requires *exactly one* field to be `True`, this constraint allows
+    multiple fields to be `True` simultaneously.
+
+    Parameters
+    ----------
+    *field_names : str
+        Varargs list of at least two unique field names.
+
+    Returns
+    -------
+    Callable
+        Decorator factory
+
+    Example
+    -------
+    >>> from pydantic import BaseModel, ValidationError
+    >>>
+    >>> @require_any_true("foo", "bar")
+    ... class MyModel(BaseModel):
+    ...     foo: bool | None = None
+    ...     bar: bool = True
+    ...
+    >>> MyModel()                       # validates OK
+    MyModel(foo=None, bar=True)
+    >>> MyModel(foo=True, bar=True)     # validates OK
+    MyModel(foo=True, bar=True)
+    >>> MyModel(foo=True, bar=False)    # validates OK
+    MyModel(foo=True, bar=False)
+    >>>
+    >>> try:
+    ...     MyModel(bar=False)
+    ... except ValidationError as e:
+    ...    assert (
+    ...        "at least one field from the `bool` field group [foo, bar] must be True, "
+    ...        "but none is True"
+    ...    ) in str(e)
+    ...    print("Validation failed")
+    Validation failed
+    """
+    model_constraint = RequireAnyTrueConstraint._create_internal(
+        f"@{require_any_true.__name__}", *field_names
+    )
+
+    return model_constraint.decorate
+
+
+class RequireAnyTrueConstraint(FieldGroupConstraint):
+    """
+    Class implementing the `require_any_true` decorator, which can also be used standalone.
+    """
+
+    def __init__(self, *field_names: str):
+        super().__init__(
+            None, RequireAnyTrueConstraint.__validate_field_names(field_names)
+        )
+
+    @classmethod
+    def _create_internal(
+        cls, name: str, *field_names: str
+    ) -> "RequireAnyTrueConstraint":
+        instance = cls.__new__(cls)
+        super(RequireAnyTrueConstraint, instance).__init__(
+            name, RequireAnyTrueConstraint.__validate_field_names(field_names)
+        )
+        return instance
+
+    @staticmethod
+    def __validate_field_names(field_names: tuple[str, ...]) -> tuple[str, ...]:
+        if len(field_names) < 2:
+            raise ValueError(
+                f"`field_names` must contain at least two items, but {field_names} has only {len(field_names)}"
+            )
+        return field_names
+
+    @override
+    def validate_class(self, model_class: type[BaseModel]) -> None:
+        super().validate_class(model_class)
+
+        def is_bool(annotation: type[Any] | None) -> bool:
+            if annotation is bool:
+                return True
+            origin = get_origin(annotation)
+            if origin is Annotated:
+                return is_bool(get_args(annotation)[0])
+            elif get_origin(annotation) in (Union, UnionType):
+                args = get_args(annotation)
+                return any(is_bool(a) for a in args) and all(
+                    is_bool(a) or a in (None, NoneType) for a in args
+                )
+            else:
+                return False
+
+        non_bool_fields = [
+            f
+            for f in self.field_names
+            if not is_bool(model_class.model_fields[f].annotation)
+        ]
+        if non_bool_fields:
+            raise TypeError(
+                f"`{self.name}` specifies fields that are have a non-`bool` type in the model class `{model_class.__name__}`: {', '.join(non_bool_fields)}  "
+            )
+
+    @override
+    def validate_instance(self, model_instance: BaseModel) -> None:
+        super().validate_instance(model_instance)
+
+        true_fields = [
+            f for f in self.field_names if getattr(model_instance, f) is True
+        ]
+
+        if len(true_fields) >= 1:
+            return
+
+        raise ValueError(
+            f"at least one field from the `bool` field group [{', '.join(self.field_names)}] "
+            f"must be True, but none is True (`{self.name}`)"
+        )
+
+    @override
+    def edit_config(self, model_class: type[BaseModel], config: ConfigDict) -> None:
+        super().edit_config(model_class, config)
+
+        json_schema = get_static_json_schema_extra(config)
+
+        def has_true_value(field_name: str) -> dict:
+            return {
+                "properties": {apply_alias(model_class, field_name): {"const": True}}
+            }
+
+        put_any_of(json_schema, [has_true_value(f) for f in self.field_names])

schema/divisions/division_boundary.yaml

@@ -40,7 +40,7 @@ properties:     # JSON Schema: Top-level object properties.
         then:
           required:
             - admin_level
-    oneOf:
+    anyOf:
       - properties:
           is_land:
             const: true