Skip to content

Commit

Permalink
feat(tool): Prevent multi-merges
Browse files Browse the repository at this point in the history
Update the comparison outcome logic to not allow the user to
consolidate the same node multiple times.
  • Loading branch information
R2ZER0 committed Apr 12, 2024
1 parent 2440af6 commit 74fad30
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 17 deletions.
4 changes: 2 additions & 2 deletions tool/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions tool/model/comparison.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
3 changes: 0 additions & 3 deletions tool/model/consolidation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 12 additions & 2 deletions tool/model/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"<Node {name}>"
return f"<Node {self.name}>"


class Span(Feature):
Expand Down
4 changes: 3 additions & 1 deletion tool/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 48 additions & 0 deletions tool/view_warningbox.py
Original file line number Diff line number Diff line change
@@ -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
70 changes: 61 additions & 9 deletions tool/viewmodel/state.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 74fad30

Please sign in to comment.