QGIS plugin updates for node_type, remove explode_and_connect (#1220

)

@JvanHouwelingen noticed that we don't create a `node_id` column instead
of `fid`. To be able to edit tables in QGIS we always need `fid`, but
its value no longer matters, so leaving it to autogenerate is normally
best.

This removes `explode_and_connect` to automatically fill in part of the
Edge attributes when creating a new geometry. It was no longer correct
due to the schema updates, so better to remove it for now, and accept
that users have to both draw the geometry and fill in the correct
attributes. An issue with the desired behavior is already written in
#352. In short, `derive_connectivity` would still be nice to have, but
not `explode`.

- add `node_id` to the Node schema
- use `node_id` in the Node label
- add `from_node_type` and `to_node_type` to the Edge schema
- keep showing fid in the Edge label to be able to associate edge_id in
timeseries widget
- use a dropdown selector for the Node and Edge editor widget, see
screenshot below
- update schemas with `listen_node_type` as in #1110


![image](https://github.com/Deltares/Ribasim/assets/4471859/aae55ec1-0612-408f-864b-e1158af8fe28)

Edit: with the updates from Huite this fixes #352

---------

Co-authored-by: Huite Bootsma <[email protected]>

ribasim_qgis/core/nodes.py

@@ -113,6 +113,16 @@ def set_defaults(self) -> None:
             index = fields.indexFromName(name)
             self.layer.setDefaultValueDefinition(index, definition)
 
+    def set_dropdown(self, name: str, options: set[str]) -> None:
+        """Use a dropdown menu for a field in the editor widget."""
+        layer = self.layer
+        index = layer.fields().indexFromName(name)
+        setup = QgsEditorWidgetSetup(
+            "ValueMap",
+            {"map": {node: node for node in options}},
+        )
+        layer.setEditorWidgetSetup(index, setup)
+
     def set_read_only(self) -> None:
         pass
 
@@ -160,6 +170,7 @@ def attributes(cls) -> list[QgsField]:
         return [
             QgsField("name", QVariant.String),
             QgsField("node_type", QVariant.String),
+            QgsField("node_id", QVariant.Int),
             QgsField("subnetwork_id", QVariant.Int),
         ]
 
@@ -177,15 +188,11 @@ def write(self) -> None:
 
     def set_editor_widget(self) -> None:
         layer = self.layer
-        index = layer.fields().indexFromName("node_type")
-        setup = QgsEditorWidgetSetup(
-            "ValueMap",
-            {"map": {node: node for node in NONSPATIALNODETYPES}},
-        )
-        layer.setEditorWidgetSetup(index, setup)
+        node_type_field = layer.fields().indexFromName("node_type")
+        self.set_dropdown("node_type", NONSPATIALNODETYPES)
 
         layer_form_config = layer.editFormConfig()
-        layer_form_config.setReuseLastValue(1, True)
+        layer_form_config.setReuseLastValue(node_type_field, True)
         layer.setEditFormConfig(layer_form_config)
 
         return
@@ -243,7 +250,7 @@ def renderer(self) -> QgsCategorizedSymbolRenderer:
     @property
     def labels(self) -> Any:
         pal_layer = QgsPalLayerSettings()
-        pal_layer.fieldName = """concat("name", ' #', "fid")"""
+        pal_layer.fieldName = """concat("name", ' #', "node_id")"""
         pal_layer.isExpression = True
         pal_layer.dist = 2.0
         labels = QgsVectorLayerSimpleLabeling(pal_layer)
@@ -255,7 +262,9 @@ class Edge(Input):
     def attributes(cls) -> list[QgsField]:
         return [
             QgsField("name", QVariant.String),
+            QgsField("from_node_type", QVariant.String),
             QgsField("from_node_id", QVariant.Int),
+            QgsField("to_node_type", QVariant.String),
             QgsField("to_node_id", QVariant.Int),
             QgsField("edge_type", QVariant.String),
             QgsField("subnetwork_id", QVariant.Int),
@@ -275,15 +284,12 @@ def is_spatial(cls):
 
     def set_editor_widget(self) -> None:
         layer = self.layer
-        index = layer.fields().indexFromName("edge_type")
-        setup = QgsEditorWidgetSetup(
-            "ValueMap",
-            {"map": {node: node for node in EDGETYPES}},
-        )
-        layer.setEditorWidgetSetup(index, setup)
+
+        self.set_dropdown("edge_type", EDGETYPES)
+        self.set_dropdown("from_node_type", NONSPATIALNODETYPES)
+        self.set_dropdown("to_node_type", NONSPATIALNODETYPES)
 
         layer_form_config = layer.editFormConfig()
-        layer_form_config.setReuseLastValue(1, True)
         layer.setEditFormConfig(layer_form_config)
 
         return
@@ -697,7 +703,8 @@ def geometry_type(cls) -> str:
     def attributes(cls) -> list[QgsField]:
         return [
             QgsField("node_id", QVariant.Int),
-            QgsField("listen_feature_id", QVariant.Int),
+            QgsField("listen_node_type", QVariant.String),
+            QgsField("listen_node_id", QVariant.Int),
             QgsField("variable", QVariant.String),
             QgsField("greater_than", QVariant.Double),
         ]
@@ -735,6 +742,7 @@ def attributes(cls) -> list[QgsField]:
         return [
             QgsField("node_id", QVariant.Int),
             QgsField("active", QVariant.Bool),
+            QgsField("listen_node_type", QVariant.String),
             QgsField("listen_node_id", QVariant.Int),
             QgsField("target", QVariant.Double),
             QgsField("proportional", QVariant.Double),
@@ -756,6 +764,7 @@ def geometry_type(cls) -> str:
     def attributes(cls) -> list[QgsField]:
         return [
             QgsField("node_id", QVariant.Int),
+            QgsField("listen_node_type", QVariant.String),
             QgsField("listen_node_id", QVariant.Int),
             QgsField("time", QVariant.DateTime),
             QgsField("target", QVariant.Double),
@@ -850,6 +859,7 @@ def attributes(cls) -> list[QgsField]:
     cls.nodetype() for cls in Input.__subclasses__() if not cls.is_spatial()
 }
 EDGETYPES = {"flow", "control"}
+SPATIALCONTROLNODETYPES = {"DiscreteControl", "PidControl"}
 
 
 def load_nodes_from_geopackage(path: Path) -> dict[str, Input]:

ribasim_qgis/core/topology.py

@@ -2,6 +2,10 @@
 from typing import TYPE_CHECKING, cast
 
 import numpy as np
+from qgis.core import QgsFeature, QgsVectorLayer
+from qgis.core.additions.edit import edit
+
+from ribasim_qgis.core.nodes import SPATIALCONTROLNODETYPES
 
 if TYPE_CHECKING:
     from numpy.typing import NDArray
@@ -10,39 +14,6 @@
 
     NDArray: type = Sequence
 
-# qgis is monkey patched by plugins.processing.
-# Importing from plugins directly for mypy
-from plugins import processing
-from qgis.core import QgsFeature, QgsVectorLayer
-from qgis.core.additions.edit import edit
-
-
-def explode_lines(edge: QgsVectorLayer) -> None:
-    args = {
-        "INPUT": edge,
-        "OUTPUT": "memory:",
-    }
-    memory_layer = processing.run("native:explodelines", args)["OUTPUT"]
-
-    # Now overwrite the contents of the original layer.
-    try:
-        # Avoid infinite recursion and stackoverflow
-        edge.blockSignals(True)
-        provider = edge.dataProvider()
-        assert provider is not None
-
-        with edit(edge):
-            edge_iterator = cast(Iterable[QgsFeature], edge.getFeatures())
-            provider.deleteFeatures([f.id() for f in edge_iterator])
-            new_features = list(memory_layer.getFeatures())
-            for i, feature in enumerate(new_features):
-                feature["fid"] = i + 1
-            provider.addFeatures(new_features)
-    finally:
-        edge.blockSignals(False)
-
-    return
-
 
 def derive_connectivity(
     node_index: NDArray[np.int_],
@@ -52,13 +23,12 @@ def derive_connectivity(
     """
     Derive connectivity on the basis of xy locations.
 
-    If the edges have been setup neatly through snapping in QGIS, the points
-    should be the same.
+    If the first and last vertices of the edges have been setup neatly through
+    snapping in QGIS, the points should be the same.
     """
     # collect xy
     # stack all into a single array
     xy = np.vstack([node_xy, edge_xy])
-    _, inverse = np.unique(xy, return_inverse=True, axis=0)
     _, index, inverse = np.unique(xy, return_index=True, return_inverse=True, axis=0)
     uniques_index = index[inverse]
 
@@ -73,3 +43,109 @@ def derive_connectivity(
     from_id = node_index[edge_node_id[:, 0]]
     to_id = node_index[edge_node_id[:, 1]]
     return from_id, to_id
+
+
+def collect_node_properties(
+    node: QgsVectorLayer,
+) -> tuple[NDArray[np.float64], NDArray[np.int_], dict[str, tuple[str, int]]]:
+    n_node = node.featureCount()
+    node_fields = node.fields()
+    type_field = node_fields.indexFromName("node_type")
+    id_field = node_fields.indexFromName("node_id")
+
+    node_xy = np.empty((n_node, 2), dtype=float)
+    node_index = np.empty(n_node, dtype=int)
+    node_iterator = cast(Iterable[QgsFeature], node.getFeatures())
+    node_identifiers = {}
+    for i, feature in enumerate(node_iterator):
+        point = feature.geometry().asPoint()
+        node_xy[i, 0] = point.x()
+        node_xy[i, 1] = point.y()
+        feature_id = feature.attribute(0)
+        node_index[i] = feature_id
+        node_type = feature.attribute(type_field)
+        node_id = feature.attribute(id_field)
+        node_identifiers[feature_id] = (node_type, node_id)
+
+    return node_xy, node_index, node_identifiers
+
+
+def collect_edge_coordinates(edge: QgsVectorLayer) -> NDArray[np.float64]:
+    # Collect the coordinates of the first and last vertex of every edge
+    # geometry.
+    n_edge = edge.featureCount()
+    edge_xy = np.empty((n_edge, 2, 2), dtype=float)
+    edge_iterator = cast(Iterable[QgsFeature], edge.getFeatures())
+    for i, feature in enumerate(edge_iterator):
+        geometry = feature.geometry().asPolyline()
+        first = geometry[0]
+        last = geometry[-1]
+        edge_xy[i, 0, 0] = first.x()
+        edge_xy[i, 0, 1] = first.y()
+        edge_xy[i, 1, 0] = last.x()
+        edge_xy[i, 1, 1] = last.y()
+    edge_xy = edge_xy.reshape((-1, 2))
+    return edge_xy
+
+
+def infer_edge_type(from_node_type: str) -> str:
+    if from_node_type in SPATIALCONTROLNODETYPES:
+        return "control"
+    else:
+        return "flow"
+
+
+def set_edge_properties(node: QgsVectorLayer, edge: QgsVectorLayer) -> None:
+    """
+    Based on the location of the first and last vertex of every edge geometry,
+    derive which nodes it connects.
+
+    Sets values for:
+
+    * from_node_type
+    * from_node_id
+    * to_node_type
+    * to_node_id
+    * edge_type
+
+    """
+    node_xy, node_index, node_identifiers = collect_node_properties(node)
+    edge_xy = collect_edge_coordinates(edge)
+    from_fid, to_fid = derive_connectivity(node_index, node_xy, edge_xy)
+
+    edge_fields = edge.fields()
+    from_type_field = edge_fields.indexFromName("from_node_type")
+    from_id_field = edge_fields.indexFromName("from_node_id")
+    to_type_field = edge_fields.indexFromName("to_node_type")
+    to_id_field = edge_fields.indexFromName("to_node_id")
+    edge_type_field = edge_fields.indexFromName("edge_type")
+
+    try:
+        # Avoid infinite recursion
+        edge.blockSignals(True)
+        with edit(edge):
+            edge_iterator = cast(Iterable[QgsFeature], edge.getFeatures())
+            for feature, fid1, fid2 in zip(edge_iterator, from_fid, to_fid):
+                type1, id1 = node_identifiers[fid1]
+                type2, id2 = node_identifiers[fid2]
+                edge_type = infer_edge_type(type1)
+
+                fid = feature.id()
+                edge.changeAttributeValue(
+                    fid,
+                    from_type_field,
+                    type1,
+                )
+                edge.changeAttributeValue(fid, from_id_field, id1)
+                edge.changeAttributeValue(
+                    fid,
+                    to_type_field,
+                    type2,
+                )
+                edge.changeAttributeValue(fid, to_id_field, id2)
+                edge.changeAttributeValue(fid, edge_type_field, edge_type)
+
+    finally:
+        edge.blockSignals(False)
+
+    return

ribasim_qgis/widgets/dataset_widget.py

@@ -4,14 +4,13 @@
 This widget also allows enabling or disabling individual elements for a
 computation.
 """
+
 from __future__ import annotations
 
-from collections.abc import Iterable
 from datetime import datetime
 from pathlib import Path
 from typing import Any, cast
 
-import numpy as np
 from PyQt5.QtCore import Qt
 from PyQt5.QtWidgets import (
     QAbstractItemView,
@@ -29,19 +28,17 @@
     QWidget,
 )
 from qgis.core import (
-    QgsFeature,
     QgsMapLayer,
     QgsProject,
     QgsVectorLayer,
 )
-from qgis.core.additions.edit import edit
 
 from ribasim_qgis.core.model import (
     get_database_path_from_model_file,
     get_directory_path_from_model_file,
 )
 from ribasim_qgis.core.nodes import Edge, Input, Node, load_nodes_from_geopackage
-from ribasim_qgis.core.topology import derive_connectivity, explode_lines
+from ribasim_qgis.core.topology import set_edge_properties
 
 
 class DatasetTreeWidget(QTreeWidget):
@@ -174,53 +171,14 @@ def path(self) -> Path:
         """Returns currently active path to Ribasim model (.toml)"""
         return Path(self.dataset_line_edit.text())
 
-    def explode_and_connect(self) -> None:
+    def connect_nodes(self) -> None:
         node = self.node_layer
         edge = self.edge_layer
         assert edge is not None
         assert node is not None
-        explode_lines(edge)
-
-        n_node = node.featureCount()
-        n_edge = edge.featureCount()
-        if n_node == 0 or n_edge == 0:
-            return
 
-        node_xy = np.empty((n_node, 2), dtype=float)
-        node_index = np.empty(n_node, dtype=int)
-        node_iterator = cast(Iterable[QgsFeature], node.getFeatures())
-        for i, feature in enumerate(node_iterator):
-            point = feature.geometry().asPoint()
-            node_xy[i, 0] = point.x()
-            node_xy[i, 1] = point.y()
-            node_index[i] = feature.attribute(0)  # Store the feature id
-
-        edge_xy = np.empty((n_edge, 2, 2), dtype=float)
-        edge_iterator = cast(Iterable[QgsFeature], edge.getFeatures())
-        for i, feature in enumerate(edge_iterator):
-            geometry = feature.geometry().asPolyline()
-            for j, point in enumerate(geometry):
-                edge_xy[i, j, 0] = point.x()
-                edge_xy[i, j, 1] = point.y()
-        edge_xy = edge_xy.reshape((-1, 2))
-        from_id, to_id = derive_connectivity(node_index, node_xy, edge_xy)
-
-        fields = edge.fields()
-        field1 = fields.indexFromName("from_node_id")
-        field2 = fields.indexFromName("to_node_id")
-        try:
-            # Avoid infinite recursion
-            edge.blockSignals(True)
-            with edit(edge):
-                edge_iterator = cast(Iterable[QgsFeature], edge.getFeatures())
-                for feature, id1, id2 in zip(edge_iterator, from_id, to_id):
-                    fid = feature.id()
-                    # Nota bene: will fail with numpy integers, has to be Python type!
-                    edge.changeAttributeValue(fid, field1, int(id1))
-                    edge.changeAttributeValue(fid, field2, int(id2))
-
-        finally:
-            edge.blockSignals(False)
+        if (node.featureCount() > 0) and (edge.featureCount() > 0):
+            set_edge_properties(node, edge)
 
         return
 
@@ -287,7 +245,7 @@ def load_geopackage(self) -> None:
         # Connect node and edge layer to derive connectivities.
         self.node_layer = node.layer
         self.edge_layer = edge.layer
-        self.edge_layer.editingStopped.connect(self.explode_and_connect)
+        self.edge_layer.editingStopped.connect(self.connect_nodes)
         return
 
     def new_model(self) -> None: