From 74fad30eec997da65dd0f27888d770a7e333d88f Mon Sep 17 00:00:00 2001 From: Rikki Guy Date: Thu, 11 Apr 2024 19:49:35 +0100 Subject: [PATCH] feat(tool): Prevent multi-merges Update the comparison outcome logic to not allow the user to consolidate the same node multiple times. --- tool/control.py | 4 +-- tool/model/comparison.py | 6 ++++ tool/model/consolidation.py | 3 -- tool/model/network.py | 14 ++++++-- tool/view.py | 4 ++- tool/view_warningbox.py | 48 +++++++++++++++++++++++++ tool/viewmodel/state.py | 70 ++++++++++++++++++++++++++++++++----- 7 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 tool/view_warningbox.py diff --git a/tool/control.py b/tool/control.py index b66b2ed..db0f04a 100644 --- a/tool/control.py +++ b/tool/control.py @@ -101,8 +101,8 @@ def onPrevButton(self, state: ToolState) -> ToolState: def onSameButton(self, state: ToolState) -> ToolState: if isinstance(state, ToolNodeComparisonState): - state.setOutcomeConsolidate() - state.gotoNextComparison() + if state.setOutcomeConsolidate(): + state.gotoNextComparison() return state else: diff --git a/tool/model/comparison.py b/tool/model/comparison.py index 97e3e05..39640a5 100644 --- a/tool/model/comparison.py +++ b/tool/model/comparison.py @@ -221,6 +221,12 @@ def distance_km(self): return self._point_distance_km(point_a, point_b) + def __eq__(self, value: object) -> bool: + if isinstance(value, NodeComparison): + return self.node_a == value.node_a and self.node_b == value.node_b + else: + raise ValueError("Can't test equality with non-NodeComparison") + @dataclass class SpanComparison(Comparison): diff --git a/tool/model/consolidation.py b/tool/model/consolidation.py index c5efe03..2c64cf2 100644 --- a/tool/model/consolidation.py +++ b/tool/model/consolidation.py @@ -43,9 +43,6 @@ def _compare_nodes(self): Create NodeComparisons, and check for either auto-merging or give to the UI to ask the user. """ - # TODO - this compares every node with every other node - # - needs replacing with code to only compare to nearest neighbours instead - for a_node in self.network_a.nodes: for b_node in self.network_b.nodes: comparison = NodeComparison(a_node, b_node) diff --git a/tool/model/network.py b/tool/model/network.py index 190fdb5..bb180d6 100644 --- a/tool/model/network.py +++ b/tool/model/network.py @@ -39,6 +39,12 @@ def __hash__(self) -> int: # Enable Nodes/Spans to be put in a Set or Dict return hash(self.id) + def __eq__(self, other: object) -> bool: + if isinstance(other, Feature): + return self.id == other.id + else: + raise ValueError("Can't compare Feature to non-Feature") + class Node(Feature): featureType = FeatureType.NODE @@ -104,9 +110,13 @@ def get(self, k): return self.properties.get(k) + @property + def name(self) -> str: + """Human readable name""" + return self.properties["name"] if "name" in self.properties else self.id + def __str__(self): - name = self.properties["name"] if "name" in self.properties else self.id - return f"" + return f"" class Span(Feature): diff --git a/tool/view.py b/tool/view.py index 3caa210..91f8871 100644 --- a/tool/view.py +++ b/tool/view.py @@ -378,7 +378,9 @@ def _updateComparing(self, state: ToolNodeComparisonState): self.prevButton.setEnabled(True) self.progressLabel.setText( - "Nodes" if state.state == ToolStateEnum.COMPARING_NODES else "Spans" + f"Node Comparison {state.current+1} of {state.nTotal}" + if state.state == ToolStateEnum.COMPARING_NODES + else "Span" ) self.progressBar.setEnabled(True) self.progressBar.setMinimum(0) diff --git a/tool/view_warningbox.py b/tool/view_warningbox.py new file mode 100644 index 0000000..2a87cb1 --- /dev/null +++ b/tool/view_warningbox.py @@ -0,0 +1,48 @@ +from typing import Literal + +from PyQt5.QtWidgets import QMessageBox +from .model.network import Node + + +def show_node_consolidation_warning( + a_or_b: Literal["A", "B"], node: Node, other_node: Node +) -> bool: + """ + This is a popup warning box shown to users when they try to consolidate a node that + has already been consolidated in a different comparison. + + Returns True/False, True if the user clicks OK, False if Cancel. + + This totally breaks the MVVM / MVC pattern we're using, but it works. + """ + user_says_ok = False + + msg = QMessageBox() + msg.setModal(True) + msg.setIcon(QMessageBox.Icon.Question) + msg.setWindowTitle(f"Node {a_or_b} {node.name} already consolidated") + msg.setInformativeText( + ( + f"The node {a_or_b} {node.name} (id = {node.id}" + + " has already been marked as the same as another node" + + f" {other_node.name} (id = {other_node.id})).\n" + + f" Are you sure you want to mark node {a_or_b} {node.name} " + + f" (id = {node.id}) as the same here instead?" + ) + ) + msg.setStandardButtons( + QMessageBox.StandardButton.Ok | QMessageBox.StandardButton.Cancel + ) + + def _btn_clicked(btn): + nonlocal user_says_ok + if btn.text() == "OK": + user_says_ok = True + else: + user_says_ok = False + + msg.buttonClicked.connect(_btn_clicked) + + msg.exec_() + + return user_says_ok diff --git a/tool/viewmodel/state.py b/tool/viewmodel/state.py index d845c4c..752f36e 100644 --- a/tool/viewmodel/state.py +++ b/tool/viewmodel/state.py @@ -1,11 +1,13 @@ from typing import ClassVar, List, Union, Tuple from enum import Enum + from qgis.core import QgsVectorLayer from ..model.consolidation import NetworkNodesConsolidator from ..model.comparison import ConsolidationReason, NodeComparison, ComparisonOutcome -from ..model.network import Network +from ..model.network import Network, Node +from ..view_warningbox import show_node_consolidation_warning class ToolInvalidState(Exception): @@ -70,33 +72,83 @@ def __str__(self): ) def gotoNextComparison(self): - new_current = self.current + 1 - if new_current >= self.nTotal: - new_current = 0 - self.current = new_current + self.current += 1 + if self.current >= self.nTotal: + self.current = 0 def gotoPrevComparison(self): - new_current = self.current - 1 + self.current -= 1 if self.current < 0: - new_current = self.nTotal - 1 - self.current = new_current + self.current = self.nTotal - 1 - def setOutcomeConsolidate(self): + def setOutcomeConsolidate(self) -> bool: + """ + Set the outcome of the current comparison as The Same aka Consolidate. + Returns False if the user cancelled, True if was successful. + """ comparison = self.currentComparison assert comparison is not None + + # Find all other comparisons containing one of the now-consolidated nodes, + # and set them to "Not Same", because we can only consolidate a node once. + other_comparisons_with_node_a: List[Tuple[int, Node]] = list() + other_comparisons_with_node_b: List[Tuple[int, Node]] = list() + for other_i in range(len(self.comparisons_outcomes)): + (other_comparison, other_outcome) = self.comparisons_outcomes[other_i] + + if comparison == other_comparison: + continue + + if comparison.node_a.id == other_comparison.node_a.id: + other_comparisons_with_node_a.append((other_i, other_comparison.node_b)) + + elif comparison.node_b.id == other_comparison.node_b.id: + other_comparisons_with_node_b.append((other_i, other_comparison.node_a)) + + # Check that none of the other comparisons w/ overlapping nodes have already + # been chosen to consolidate, if so display a warning to the user + + for other_i, other_node in other_comparisons_with_node_a: + (other_comparison, other_outcome) = self.comparisons_outcomes[other_i] + if other_outcome is not None and other_outcome.consolidate is not False: + if not show_node_consolidation_warning( + "A", comparison.node_a, other_node + ): + return False + + for other_i, other_node in other_comparisons_with_node_b: + (other_comparison, other_outcome) = self.comparisons_outcomes[other_i] + if other_outcome is not None and other_outcome.consolidate is not False: + if not show_node_consolidation_warning( + "B", comparison.node_b, other_node + ): + return False + + # If we've got this far, update all the other comparisons to be "not same" + for other_i, _ in other_comparisons_with_node_a + other_comparisons_with_node_b: + self.comparisons_outcomes[other_i] = ( + other_comparison, + ComparisonOutcome(consolidate=False), + ) + + # Finally, update the outcome with a manual consolidation reason reason = ConsolidationReason( feature_type="NODE", primary_node=comparison.node_a, secondary_node=comparison.node_b, confidence=comparison.confidence, + # TODO: user's choice of matching properties? User text message? matching_properties=comparison.get_high_scoring_properties(), manual=True, ) + self.comparisons_outcomes[self.current] = ( comparison, ComparisonOutcome(consolidate=reason), ) + return True + def setOutcomeDontConsolidate(self): comparison = self.currentComparison assert comparison is not None