Fix tag editing

.github/workflows/ci.yml

@@ -15,11 +15,11 @@ jobs:
       security-events: read
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-python@v5
-        with:
-          python-version: '3.10'
-          cache: 'pip'
       - uses: psf/black@25.1.0
+      - name: unittest
+        run: |
+          sudo apt-get install -y python3-pyqt5 xvfb
+          xvfb-run -a python3 -m unittest discover -v --start-directory tests
       - name: wheel
         run: |
           pip install build

gmc/markup_objects/tags.py

@@ -220,110 +220,127 @@ def keyPressEvent(self, event: QtGui.QKeyEvent) -> None:
         super().keyPressEvent(event)
 
 
-def edit_tags(
-    parent: ImageWidget,
-    items: list[MarkupObjectMeta],
-    extra_tags: Sequence[str] = (),
-) -> None:
-    """
-    :param parent: QDialog's parent QWidget
-    :param tags: dict. tag name: number of objects with that tag
-    :param items: number of objects selected. guaranteed to be > 0
-    """
-    if not items:
-        return
-    tags: defaultdict[str, int] = defaultdict(
-        int, {tag: 0 for tag in extra_tags}
-    )
-    for item in items:
-        for tag in item.get_tags():
-            tags[tag] += 1
-
-    dialog = QtWidgets.QDialog(parent, windowTitle=tr("Edit Tags"))
-
-    def item_changed(item: QtWidgets.QListWidgetItem) -> None:
-        if (
-            item.checkState() == Qt.Checked
-            and int(QtGui.QGuiApplication.keyboardModifiers()) == Qt.ALT
-        ):
-            for i in range(tag_list_widget.count()):
-                it = tag_list_widget.item(i)
-                if it.checkState() == Qt.Checked and it != item:
-                    it.setCheckState(Qt.Unchecked)
-
-    tag_list_widget = QtWidgets.QListWidget(
-        itemChanged=item_changed, toolTip="Alt+click to check single item"
-    )
-    tags_label = QtWidgets.QLabel(tr("&Tags:"))
-    tags_label.setBuddy(tag_list_widget)
-
-    count = len(items)
-    for tag in sorted(tags):
-        item = QtWidgets.QListWidgetItem(tag, tag_list_widget)
-        item.setFlags(item.flags() | Qt.ItemIsTristate | Qt.ItemIsEditable)
-        if tags[tag] == count:
-            state = Qt.Checked
-        elif tags[tag]:
-            state = Qt.PartiallyChecked
-        else:
-            state = Qt.Unchecked
-        item.setCheckState(state)
-
-    tag_line_edit = TagEdit()
-    add_tag_label = QtWidgets.QLabel(tr("&Add Tag:"))
-    add_tag_label.setBuddy(tag_line_edit)
+class TagsDialog(QtWidgets.QDialog):
+    def __init__(
+        self,
+        parent: ImageWidget[Any],
+        items: list[HasTags],
+        extra_tags: Sequence[str] = (),
+    ) -> None:
+        """
+        :param parent: ImageWidget for staring undo actions
+        :param items: objects with "get_tags" method
+        :param extra_tags: typically a list of string from configuration file
+        """
+        super().__init__(parent, windowTitle=tr("Edit Tags"))
+        self._items = items
+        # self.setAttribute(Qt.WA_DeleteOnClose)
 
-    layout = QtWidgets.QVBoxLayout(dialog)
-    layout.addWidget(tags_label)
-    layout.addWidget(tag_list_widget)
+        tags: defaultdict[str, int] = defaultdict(
+            int, {tag: 0 for tag in extra_tags}
+        )
+        for item in items:
+            for tag in item.get_tags():
+                tags[tag] += 1
+        self._initial_tags = set(tags)
 
-    add_layout = QtWidgets.QHBoxLayout(margin=0)
-    add_layout.addWidget(tag_line_edit)
+        self._tag_list_widget = tag_list_widget = QtWidgets.QListWidget(
+            toolTip=tr("Alt+click to check single item"),
+        )
+        tags_label = QtWidgets.QLabel(tr("&Tags:"))
+        tags_label.setBuddy(tag_list_widget)
 
-    def append_tag() -> None:
-        tag = tag_line_edit.text().strip()
-        if not tag:
-            return
-        tag_line_edit.setText("")
-        existing_item = tag_list_widget.findItems(tag, Qt.MatchExactly)
-        if existing_item:
-            existing_item[0].setCheckState(Qt.Checked)
-        else:
+        count = len(items)
+        for tag in sorted(tags):
             item = QtWidgets.QListWidgetItem(tag, tag_list_widget)
-            item.setFlags(item.flags() | Qt.ItemIsTristate)
-            item.setCheckState(Qt.Checked)
-
-    tag_line_edit.commit.connect(append_tag)
-    add_layout.addWidget(
-        QtWidgets.QPushButton(
-            tr("A&ppend"), clicked=append_tag, toolTip="Ctrl+Enter, Ald+Enter"
+            item.setFlags(item.flags() | Qt.ItemIsTristate | Qt.ItemIsEditable)
+            if tags[tag] == count:
+                state = Qt.Checked
+            elif tags[tag]:
+                state = Qt.PartiallyChecked
+            else:
+                state = Qt.Unchecked
+            item.setCheckState(state)
+
+        self._tag_line_edit = tag_line_edit = TagEdit()
+        add_tag_label = QtWidgets.QLabel(tr("&Add Tag:"))
+        add_tag_label.setBuddy(tag_line_edit)
+
+        layout = QtWidgets.QVBoxLayout(self)
+        layout.addWidget(tags_label)
+        layout.addWidget(tag_list_widget)
+
+        add_layout = QtWidgets.QHBoxLayout(margin=0)
+        add_layout.addWidget(tag_line_edit)
+
+        tag_line_edit.commit.connect(self._append_tag)
+        add_layout.addWidget(
+            QtWidgets.QPushButton(
+                tr("A&ppend"),
+                clicked=self._append_tag,
+                toolTip="Ctrl+Enter, Ald+Enter",
+            )
         )
-    )
-    layout.addWidget(add_tag_label)
-    layout.addLayout(add_layout)
-
-    box = QtWidgets.QDialogButtonBox
-    button_box = box(box.Ok | box.Cancel, Qt.Horizontal, dialog)
-    button_box.accepted.connect(dialog.accept)
-    button_box.rejected.connect(dialog.reject)
-    layout.addWidget(button_box)
-    tag_line_edit.setFocus()
-
-    if dialog.exec_() == dialog.Accepted:
+        layout.addWidget(add_tag_label)
+        layout.addLayout(add_layout)
+
+        box = QtWidgets.QDialogButtonBox
+        button_box = box(box.Ok | box.Cancel, Qt.Horizontal, self)
+        button_box.accepted.connect(self._on_accept)
+        button_box.rejected.connect(self.reject)
+        layout.addWidget(button_box)
+        tag_list_widget.itemChanged.connect(self._item_changed)
+        tag_line_edit.setFocus()
+
+    def _on_accept(self):
         add, remove = [], []
-        append_tag()
+        tag_list_widget = self._tag_list_widget
+        self._append_tag()
+
+        current_tags = {
+            tag_list_widget.item(idx).text()
+            for idx in range(tag_list_widget.count())
+        }
+        remove.extend(self._initial_tags - current_tags)
+
         for idx in range(tag_list_widget.count()):
             item = tag_list_widget.item(idx)
             state = item.checkState()
             # ToDo: make code cooler by not readding tags
+            text = item.text()
             if state == Qt.Checked:
-                add.append(item.text())
+                add.append(text)
             elif state == Qt.Unchecked:
-                remove.append(item.text())
+                remove.append(text)
         if remove or add:
-            parent.scene().undo_stack.push(
-                UndoTagModification(items, add, remove)
+            self.parent().scene().undo_stack.push(
+                UndoTagModification(self._items, add, remove)
             )
+        self.accept()
+
+    def _append_tag(self) -> None:
+        tag = self._tag_line_edit.text().strip()
+        if not tag:
+            return
+        self._tag_line_edit.setText("")
+        existing_item = self._tag_list_widget.findItems(tag, Qt.MatchExactly)
+        if existing_item:
+            existing_item[0].setCheckState(Qt.Checked)
+        else:
+            item = QtWidgets.QListWidgetItem(tag, self._tag_list_widget)
+            item.setFlags(item.flags() | Qt.ItemIsTristate)
+            item.setCheckState(Qt.Checked)
+
+    def _item_changed(self, item: QtWidgets.QListWidgetItem) -> None:
+        # when user Alt clicks item, unselect all other actions
+        if (
+            item.checkState() == Qt.Checked
+            and int(QtGui.QGuiApplication.keyboardModifiers()) == Qt.ALT
+        ):
+            for i in range(self._tag_list_widget.count()):
+                it = self._tag_list_widget.item(i)
+                if it.checkState() == Qt.Checked and it != item:
+                    it.setCheckState(Qt.Unchecked)
 
 
 class UndoTagModification(QtWidgets.QUndoCommand):

gmc/schemas/fields.py

@@ -2,7 +2,7 @@
 from collections import defaultdict
 from . import MarkupSchema
 from ..markup_objects.polygon import EditableMarkupPolygon, MarkupObjectMeta
-from ..markup_objects.tags import HasTags, edit_tags
+from ..markup_objects.tags import HasTags, TagsDialog
 from ..views.image_widget import ImageWidget
 from ..utils.json import load as load_json, dump as dump_json
 from ..utils.dicts import dicts_are_equal
@@ -218,9 +218,9 @@ def _toggle_visibility(self, state):
                 item.setVisible(state)
 
     def _trigger_tag_edit(self):
-        edit_tags(
+        TagsDialog(
             self._image_widget, self._get_selected_items(), self._user_tags
-        )
+        ).exec()
 
     def _swap(self):
         for item in self._get_selected_items():

gmc/schemas/horizontal_tracking.py

@@ -2,7 +2,7 @@
 from PyQt5 import QtCore, QtGui, QtWidgets
 from . import MarkupSchema
 from ..markup_objects.polygon import MarkupObjectMeta
-from ..markup_objects.tags import HasTags, edit_tags
+from ..markup_objects.tags import HasTags, TagsDialog
 from ..views.image_widget import ImageWidget
 from ..utils.json import load as load_json, dump as dump_json
 from ..utils.dicts import dicts_are_equal
@@ -235,9 +235,9 @@ def _toggle_visibility(self, state):
                 item.setVisible(state)
 
     def _trigger_tag_edit(self):
-        edit_tags(
+        TagsDialog(
             self._image_widget, self._get_selected_items(), self._user_tags
-        )
+        ).exec()
 
     def _get_selected_items(self):
         try:

gmc/schemas/tagged_objects/__init__.py

@@ -12,7 +12,7 @@
 from ...markup_objects.line import MarkupLine
 from ...markup_objects.point import MarkupPoint
 from ...markup_objects.rect import MarkupRect
-from ...markup_objects.tags import HasTags, edit_tags, UndoTagModification
+from ...markup_objects.tags import HasTags, TagsDialog, UndoTagModification
 from ...views.image_widget import ImageWidget
 from ...utils.json import load as load_json, dump as dump_json
 from ...utils.dicts import dicts_are_equal
@@ -676,9 +676,9 @@ def _toggle_visibility(self, state) -> None:
                 item.setVisible(state)
 
     def _trigger_tag_edit(self) -> None:
-        edit_tags(
+        TagsDialog(
             self._image_widget, self._get_selected_items(), self._user_tags
-        )
+        ).exec()
 
     def _get_selected_items(self) -> list[HasTags]:
         try:

tests/test_tag_editing.py

@@ -0,0 +1,55 @@
+from __future__ import annotations
+import sys
+import unittest
+
+
+from PyQt5 import QtCore, QtWidgets
+from PyQt5.QtTest import QTest
+
+Qt = QtCore.Qt
+# QApplication must be created before AdvancedSpinbox
+_qapplication = QtWidgets.QApplication(sys.argv)
+
+from gmc.markup_objects.tags import TagsDialog
+from gmc.views.image_widget import ImageWidget
+from gmc.schemas.tagged_objects import CustomPoint
+
+
+class TestTagEditing(unittest.TestCase):
+    def _create_dialog(self, tags: list[str]):
+        image_view = ImageWidget(default_actions=[])
+        point = CustomPoint(schema=None, tags=tags)
+        dialog = TagsDialog(parent=image_view, items=[point])
+        dialog.setFocus()
+        return dialog, point, image_view
+
+    def test_add_tag_on_accept(self):
+        dialog, point, _reference = self._create_dialog(["aaa", "bbb"])
+        QTest.keyClicks(dialog._tag_line_edit, "ccc")
+        dialog._on_accept()
+        self.assertEqual(point.get_tags(), {"aaa", "bbb", "ccc"})
+
+    def test_delete_tag(self):
+        dialog, point, _reference = self._create_dialog(["aaa", "bbb"])
+        dialog._tag_list_widget.setCurrentRow(0)
+        QTest.keyClick(dialog._tag_list_widget, Qt.Key_Space)
+        dialog._on_accept()
+        self.assertEqual(point.get_tags(), {"bbb"})
+
+    def test_rename_tag(self):
+        dialog, point, _reference = self._create_dialog(["aaa", "bbb"])
+        dialog._tag_list_widget.item(0).setText("renamed")
+        dialog._on_accept()
+        self.assertEqual(point.get_tags(), {"renamed", "bbb"})
+
+    def test_all_actions(self):
+        # keep aaa
+        # rename bbb
+        # delete ccc
+        # add xxx
+        dialog, point, _reference = self._create_dialog(["aaa", "bbb", "ccc"])
+        dialog._tag_list_widget.item(1).setText("renamed")
+        dialog._tag_list_widget.takeItem(2)
+        QTest.keyClicks(dialog._tag_line_edit, "xxx")
+        dialog._on_accept()
+        self.assertEqual(point.get_tags(), {"aaa", "renamed", "xxx"})