fix(dfns): support signed index

autotest/dfns/__snapshots__/v2.0.0.dev2/gwf-sfr.json

@@ -401,7 +401,8 @@
                 "optional": true,
                 "tagged": false,
                 "dtype": "integer",
-                "index": true,
+                "index": "signed",
+                "fk": "packagedata.ifno",
                 "shape": [
                   "packagedata.ncon(ifno)"
                 ]

autotest/dfns/__snapshots__/v2.0.0.dev2/gwf-sfr.toml

@@ -365,7 +365,8 @@ description = "integer value that defines the reach number of the reach connecte
 optional = true
 tagged = false
 dtype = "integer"
-index = true
+index = "signed"
+fk = "packagedata.ifno"
 shape = [
     "packagedata.ncon(ifno)",
 ]

autotest/dfns/__snapshots__/v2.0.0.dev2/gwf-sfr.yaml

@@ -424,7 +424,8 @@ blocks:
               optional: true
               tagged: false
               dtype: integer
-              index: true
+              index: signed
+              fk: packagedata.ifno
               shape:
                 - packagedata.ncon(ifno)
   diversions:

autotest/dfns/__snapshots__/v2.0.0.dev3/gwf-sfr.json

@@ -406,7 +406,8 @@
                 "optional": true,
                 "tagged": false,
                 "dtype": "integer",
-                "index": true,
+                "index": "signed",
+                "fk": "packagedata.ifno",
                 "shape": [
                   "packagedata.ncon(ifno)"
                 ]

autotest/dfns/__snapshots__/v2.0.0.dev3/gwf-sfr.toml

@@ -368,7 +368,8 @@ description = "integer value that defines the reach number of the reach connecte
 optional = true
 tagged = false
 dtype = "integer"
-index = true
+index = "signed"
+fk = "packagedata.ifno"
 shape = [
     "packagedata.ncon(ifno)",
 ]

autotest/dfns/__snapshots__/v2.0.0.dev3/gwf-sfr.yaml

@@ -427,7 +427,8 @@ blocks:
               optional: true
               tagged: false
               dtype: integer
-              index: true
+              index: signed
+              fk: packagedata.ifno
               shape:
                 - packagedata.ncon(ifno)
   diversions:

autotest/dfns/test_migrate.py

@@ -326,3 +326,18 @@ def test_migrate_marks_cellids(dfn_dir, name, block, cols):
     for col in cols:
         field = lst.item.fields[col]
         assert (field.shape, field.index, field.cellid) == (["ncelldim"], True, True)
+
+
+def test_migrate_sfr_ic_is_signed_index(dfn_dir):
+    component = _migrate_dev3(dfn_dir, "gwf-sfr")
+    item = component.blocks["connectiondata"].fields["connectiondata"].item
+    ic = item.fields["ic"]
+    assert ic.index == "signed"
+    assert ic.fk == "packagedata.ifno"
+    # nothing else in the corpus supports negative indices
+    signed = [
+        f.name
+        for f in component.get_fields(recurse=True).values(multi=True)
+        if getattr(f, "index", False) == "signed"
+    ]
+    assert signed == ["ic"]

autotest/dfns/test_schema.py

@@ -1050,3 +1050,22 @@ def test_render_excludes_developmode_fields_by_default(dev3_spec):
     assert "CRS" not in render
     render_dev = dev3_spec.components["gwf-dis"].blocks["options"].render(developmode=True)
     assert "CRS <crs>" in render_dev
+
+
+@pytest.mark.parametrize("index", [False, True, "signed"])
+def test_index_round_trips(index):
+    for field in (
+        Integer(name="ic", index=index),
+        Array(name="ic", dtype="integer", shape=["ncon"], index=index),
+    ):
+        assert type(field).model_validate(field.model_dump()).index == index
+
+
+def test_index_rejects_unknown_kind():
+    with pytest.raises(ValueError):
+        Integer(name="ic", index="unsigned")
+
+
+def test_signed_index_requires_integer_array():
+    with pytest.raises(ValueError, match="requires dtype='integer'"):
+        Array(name="ic", dtype="double", shape=["ncon"], index="signed")

docs/md/dfnspec.md

@@ -390,7 +390,9 @@ Type `integer`.
 
 ###### `index`
 
-`boolean (default: false)`. Marks this scalar's value as a 1-based index requiring MF6's 1-based (file) <-> 0-based (Python) translation on read/write. A pure serialization fact, orthogonal to `pk`/`fk`: it does not claim the value identifies or points at a list row, only that it needs the numeric-base conversion. Not valid on `String` scalars — such fields (e.g. dynamic, name-resolved identifiers) never need this conversion.
+`boolean | "signed" (default: false)`. Marks this scalar's value as a 1-based index requiring MF6's 1-based (file) <-> 0-based (Python) translation on read/write. A pure serialization fact, orthogonal to `pk`/`fk`: it does not claim the value identifies or points at a list row, only that it needs the numeric-base conversion. Not valid on `String` scalars — such fields (e.g. dynamic, name-resolved identifiers) never need this conversion.
+
+`"signed"` marks a signed index: the value's magnitude is the 1-based index, and its sign carries a separate meaning, which only the field's description defines. There is no 0-based signed equivalent (index 0 has no negative), so a consumer must keep the sign apart from the magnitude rather than translate the value as a whole. A `fk` on a signed index applies to the magnitude.
 
 ###### `pk`
 
@@ -460,11 +462,11 @@ Dimensions are listed fastest-varying first, i.e. in the order elements are read
 
 ###### `index`
 
-`boolean (default: false)`. Marks the array's elements as 1-based indices requiring MF6's 1-based (file) <-> 0-based (Python) translation (e.g. `icvert`, `ja`, `irch`, `ievt`). Only valid when `dtype` is `"integer"`.
+`boolean | "signed" (default: false)`. Marks the array's elements as 1-based indices requiring MF6's 1-based (file) <-> 0-based (Python) translation (e.g. `icvert`, `ja`, `irch`, `ievt`). `"signed"` marks each element as a signed index, as for a scalar (e.g. SFR's `ic`). Only valid when `dtype` is `"integer"`.
 
 ###### `fk`
 
-`string | null (default: null)`. Marks the array's (nonzero) elements as a foreign key: a per-grid-cell reference to a row (by `pk`) in another list, rather than the per-list-row reference a scalar `fk` expresses (e.g. a grid-wide array giving each cell's cross-section id, referencing the cross-section package's `packagedata`). Only valid when `dtype` is `"integer"`. Hierarchical path form only (`"[component.]block.field"`) — an array has no `fk_ref` counterpart, since it has no sibling record to carry a runtime component-selector field, and no `pk` counterpart, since it has no rows of its own to be a key of. See "Primary/foreign keys".
+`string | null (default: null)`. Marks the array's (nonzero) elements as a foreign key, each a reference to a row (by `pk`) in another list. On a grid array that's a per-grid-cell reference, rather than the per-list-row reference a scalar `fk` expresses (e.g. a grid-wide array giving each cell's cross-section id, referencing the cross-section package's `packagedata`). On an array column in a list item record, each row references several rows (e.g. SFR's `ic`, the reaches connected to a reach). Only valid when `dtype` is `"integer"`. Hierarchical path form only (`"[component.]block.field"`) — an array has no `fk_ref` counterpart, since it has no sibling record to carry a runtime component-selector field, and no `pk` counterpart, since it has no rows of its own to be a key of. See "Primary/foreign keys".
 
 ###### `cellid`
 

modflow_devtools/dfn/schema.py

@@ -77,6 +77,7 @@ class Field(TypedDict):
     layered: NotRequired[bool | None]
     preserve_case: NotRequired[bool]
     numeric_index: NotRequired[bool]
+    support_negative_index: NotRequired[bool]
     # Whether MF6 requires this block's header to appear in the input file
     # even when it has zero body lines (e.g. an empty required recarray
     # block). Set explicitly on the block's aggregate field -- see

modflow_devtools/dfns/migrate_to_v2_0_0_dev2.py

@@ -577,16 +577,20 @@ def _walk(field: Any) -> Any:
 
 # Real relational facts the general `_resolve_relations`/`_mark_lonely_pk`
 # passes can't infer, backfilled by an explicit, audited allowlist rather than
-# derived. Each entry: component -> (block name, list field name, {column:
-# fk target}). Unlike `_NODE_REF_FIELDS`, block name and list field name
+# derived. Each entry: component -> [(block name, list field name, {column:
+# fk target}), ...]. Unlike `_NODE_REF_FIELDS`, block name and list field name
 # aren't assumed equal (MF6's period/perioddata idiom applies to two of these).
+# A column may be an integer scalar or an integer array (each element a key).
 #
 # - UZF's `ivertcon` is a self-referential fk (a UZF cell may point to another
 #   UZF cell below it) -- a same-block relation `_resolve_relations`
 #   explicitly never looks for.
 # - SFR's `iconr` is the downstream reach receiving diverted water -- a
 #   reference into `packagedata`'s reach numbers, not the record's leading
 #   field, so `_mark_lonely_pk` doesn't catch it either.
+# - SFR's `ic` lists the reaches connected to the current one. It's a signed
+#   index (`support_negative_index`): the sign says which end of the current
+#   reach, the magnitude which reach, so the fk applies to the magnitude.
 # - `chf-cdb`/`chf-zdg`/`olf-zdg`'s `idcxs` reference the cross-section
 #   defined by `chf-cxs`'s `packagedata.idcxs` -- a cross-component fk
 #   `_resolve_relations` structurally can't reach (single-component scope).
@@ -595,12 +599,15 @@ def _walk(field: Any) -> Any:
 #   cross-section object, so the fk target is `chf-cxs`. `chf/olf-dfw`'s
 #   `idcxs` is the same relation but is an `Array` field, not a list column
 #   -- see `_ARRAY_FK_BACKFILL` below, not here.
-_FK_BACKFILL: dict[str, tuple[str, str, dict[str, str]]] = {
-    "gwf-uzf": ("packagedata", "packagedata", {"ivertcon": "packagedata.ifno"}),
-    "gwf-sfr": ("diversions", "diversions", {"iconr": "packagedata.ifno"}),
-    "chf-cdb": ("period", "stress_period_data", {"idcxs": "chf-cxs.packagedata.idcxs"}),
-    "chf-zdg": ("period", "stress_period_data", {"idcxs": "chf-cxs.packagedata.idcxs"}),
-    "olf-zdg": ("period", "stress_period_data", {"idcxs": "chf-cxs.packagedata.idcxs"}),
+_FK_BACKFILL: dict[str, list[tuple[str, str, dict[str, str]]]] = {
+    "gwf-uzf": [("packagedata", "packagedata", {"ivertcon": "packagedata.ifno"})],
+    "gwf-sfr": [
+        ("diversions", "diversions", {"iconr": "packagedata.ifno"}),
+        ("connectiondata", "connectiondata", {"ic": "packagedata.ifno"}),
+    ],
+    "chf-cdb": [("period", "stress_period_data", {"idcxs": "chf-cxs.packagedata.idcxs"})],
+    "chf-zdg": [("period", "stress_period_data", {"idcxs": "chf-cxs.packagedata.idcxs"})],
+    "olf-zdg": [("period", "stress_period_data", {"idcxs": "chf-cxs.packagedata.idcxs"})],
 }
 
 
@@ -610,28 +617,26 @@ def _apply_fk_backfill(name: str, blocks: dict[str, v2.Block]) -> dict[str, v2.B
     See `_FK_BACKFILL` for why this is an explicit allowlist rather than a
     derived/mechanical pass.
     """
-    entry = _FK_BACKFILL.get(name)
-    if entry is None:
-        return blocks
-    block_name, list_field_name, field_targets = entry
-    block = blocks.get(block_name)
-    if block is None:
-        return blocks
-    list_field = block.fields.get(list_field_name)
-    if not isinstance(list_field, v2.List) or not isinstance(list_field.item, v2.Record):
-        return blocks
-    item = list_field.item
-    updates = {
-        fname: f.model_copy(update={"fk": target})
-        for fname, target in field_targets.items()
-        if isinstance(f := item.fields.get(fname), v2.Integer) and f.fk is None
-    }
-    if not updates:
-        return blocks
-    new_item = item.model_copy(update={"fields": {**item.fields, **updates}})
-    new_list = list_field.model_copy(update={"item": new_item})
-    new_block = block.model_copy(update={"fields": {**block.fields, list_field_name: new_list}})
-    return {**blocks, block_name: new_block}
+    for block_name, list_field_name, field_targets in _FK_BACKFILL.get(name, []):
+        block = blocks.get(block_name)
+        if block is None:
+            continue
+        list_field = block.fields.get(list_field_name)
+        if not isinstance(list_field, v2.List) or not isinstance(list_field.item, v2.Record):
+            continue
+        item = list_field.item
+        updates = {
+            fname: f.model_copy(update={"fk": target})
+            for fname, target in field_targets.items()
+            if isinstance(f := item.fields.get(fname), v2.Integer | v2.Array) and f.fk is None
+        }
+        if not updates:
+            continue
+        new_item = item.model_copy(update={"fields": {**item.fields, **updates}})
+        new_list = list_field.model_copy(update={"item": new_item})
+        new_block = block.model_copy(update={"fields": {**block.fields, list_field_name: new_list}})
+        blocks = {**blocks, block_name: new_block}
+    return blocks
 
 
 # `chf-dfw`/`olf-dfw`'s `idcxs` is a per-cell grid array (`griddata` block,
@@ -1455,8 +1460,11 @@ def _map_field(f: v1.Field) -> v2.InputField:
         layered: bool = try_parse_bool(f.get("layered"), False)
         # Pure serialization fact (1-based/0-based conversion), orthogonal to pk/fk
         # relational identity. Direct copy from v1: valid on Integer/Array(dtype=
-        # "integer") only, never on String.
-        numeric_index: bool = try_parse_bool(f.get("numeric_index"), False)
+        # "integer") only, never on String. v1's `support_negative_index` (only
+        # SFR's `ic`) means the sign carries meaning and the magnitude is the index.
+        numeric_index: v2.Index = try_parse_bool(f.get("numeric_index"), False)
+        if numeric_index and try_parse_bool(f.get("support_negative_index"), False):
+            numeric_index = "signed"
         removed: str | None = f.get("removed") or None
         deprecated: str | None = f.get("deprecated") or None
         valid = f.get("valid")

modflow_devtools/dfns/schema.json

@@ -123,9 +123,17 @@
           "type": "boolean"
         },
         "index": {
+          "anyOf": [
+            {
+              "type": "boolean"
+            },
+            {
+              "const": "signed",
+              "type": "string"
+            }
+          ],
           "default": false,
-          "title": "Index",
-          "type": "boolean"
+          "title": "Index"
         },
         "fk": {
           "anyOf": [
@@ -635,9 +643,17 @@
           "type": "boolean"
         },
         "index": {
+          "anyOf": [
+            {
+              "type": "boolean"
+            },
+            {
+              "const": "signed",
+              "type": "string"
+            }
+          ],
           "default": false,
-          "title": "Index",
-          "type": "boolean"
+          "title": "Index"
         },
         "pk": {
           "default": false,

modflow_devtools/dfns/schema.py

@@ -90,11 +90,17 @@ class String(InputFieldBase):
     fk_ref: str | None = None
 
 
+# `True`: a 1-based index in the file, 0-based in memory. `"signed"`: the
+# magnitude is such an index, and the sign carries a meaning of its own, which
+# only the field's description defines (e.g. SFR's `ic`).
+Index = bool | Literal["signed"]
+
+
 class Integer(InputFieldBase):
     type: Literal["integer"] = PydanticField(default="integer", frozen=True)
     valid: list[int] | None = None
     time_series: bool = False
-    index: bool = False
+    index: Index = False
     pk: bool = False
     fk: str | None = None
     fk_ref: str | None = None
@@ -122,7 +128,7 @@ class Array(InputFieldBase):
     shape: list[str] = []
     time_series: bool = False
     layered: bool = False
-    index: bool = False
+    index: Index = False
     fk: str | None = None
     # A cellid, or several: a grid cell reference resolved against the grid
     # the column refers to (DIS/DISV/DISU). The first (fastest-varying) axis
@@ -133,7 +139,8 @@ class Array(InputFieldBase):
     def _check_index_dtype(self) -> "Array":
         if self.index and self.dtype != "integer":
             raise ValueError(
-                f"Array {self.name!r}: index=True requires dtype='integer', got {self.dtype!r}"
+                f"Array {self.name!r}: index={self.index!r} requires dtype='integer', "
+                f"got {self.dtype!r}"
             )
         if self.cellid and self.dtype != "integer":
             raise ValueError(