Do and Undo (#84)

* Very fist Undo Command working

* Completed column alteration

* More stable undo redo of a singular column

* adding and deleting rows is now reversible

* Undo Redo provisourically complete, testing remains

* More unified petab linting

* Casting into correct datatypes now. Slight changes to error messages

* Various fixes:
1. Copy pasting is one big undo/redo
2. Setting an invalid index has a new overwrite message and sets a default value
3. Ordering of functionality is changes such that a row is only checked when necessary
4. In parameter case, now accounts for potentially more parameters than were thrown into the model

* Worked in suggestions

Author: PaulJonasJost

src/petab_gui/C.py

@@ -173,3 +173,8 @@
     "condition": DEFAULT_COND_CONFIG,
     "measurement": DEFAULT_MEAS_CONFIG
 }
+
+COMMON_ERRORS = {
+    r"Error parsing '': Syntax error at \d+:\d+: mismatched input '<EOF>' "
+    r"expecting \{[^}]+\}" : "Invalid empty cell!"
+}

src/petab_gui/commands.py

@@ -0,0 +1,200 @@
+"""Store commands for the do/undo functionality."""
+from PySide6.QtGui import QUndoCommand
+from PySide6.QtCore import QModelIndex, Qt
+import pandas as pd
+import numpy as np
+
+
+pd.set_option('future.no_silent_downcasting', True)
+
+
+class ModifyColumnCommand(QUndoCommand):
+    """Command to add a column to the table."""
+
+    def __init__(self, model, column_name, add_mode: bool = True):
+        action = "Add" if add_mode else "Remove"
+        super().__init__(
+            f"{action} column {column_name} in table {model.table_type}"
+        )
+        self.model = model
+        self.column_name = column_name
+        self.add_mode = add_mode
+        self.old_values = None
+        self.position = None
+
+        if not add_mode and column_name in model._data_frame.columns:
+            self.position = model._data_frame.columns.get_loc(column_name)
+            self.old_values = model._data_frame[column_name].copy()
+
+    def redo(self):
+        if self.add_mode:
+            position = self.model._data_frame.shape[1]
+            self.model.beginInsertColumns(QModelIndex(), position, position)
+            self.model._data_frame[self.column_name] = ""
+            self.model.endInsertColumns()
+        else:
+            self.position = self.model._data_frame.columns.get_loc(self.column_name)
+            self.model.beginRemoveColumns(QModelIndex(), self.position, self.position)
+            self.model._data_frame.drop(columns=self.column_name, inplace=True)
+            self.model.endRemoveColumns()
+
+    def undo(self):
+        if self.add_mode:
+            position = self.model._data_frame.columns.get_loc(self.column_name)
+            self.model.beginRemoveColumns(QModelIndex(), position, position)
+            self.model._data_frame.drop(columns=self.column_name, inplace=True)
+            self.model.endRemoveColumns()
+        else:
+            self.model.beginInsertColumns(QModelIndex(), self.position, self.position)
+            self.model._data_frame.insert(self.position, self.column_name, self.old_values)
+            self.model.endInsertColumns()
+
+
+class ModifyRowCommand(QUndoCommand):
+    """Command to add a row to the table."""
+
+    def __init__(
+        self,
+        model,
+        row_indices: list[int] | int,
+        add_mode: bool = True
+    ):
+        action = "Add" if add_mode else "Remove"
+        super().__init__(f"{action} row(s) in table {model.table_type}")
+        self.model = model
+        self.add_mode = add_mode
+        self.old_rows = None
+        self.old_ind_names = None
+
+        df = self.model._data_frame
+
+        if add_mode:
+            # Adding: interpret input as count of new rows
+            self.row_indices = self._generate_new_indices(row_indices)
+        else:
+            # Deleting: interpret input as specific index labels
+            self.row_indices = row_indices if isinstance(row_indices, list) else [row_indices]
+            self.old_rows = df.iloc[self.row_indices].copy()
+            self.old_ind_names = [df.index[idx] for idx in self.row_indices]
+
+    def _generate_new_indices(self, count):
+        """Generate default row indices based on table type and index type."""
+        df = self.model._data_frame
+        base = 0
+        existing = set(df.index.astype(str))
+
+        indices = []
+        while len(indices) < count:
+            idx = f"new_{self.model.table_type}_{base}"
+            if idx not in existing:
+                indices.append(idx)
+            base += 1
+        self.old_ind_names = indices
+        return indices
+
+    def redo(self):
+        df = self.model._data_frame
+
+        if self.add_mode:
+            position = df.shape[0] - 1  # insert *before* the auto-row
+            self.model.beginInsertRows(QModelIndex(), position, position + len(self.row_indices) - 1)
+            for i, idx in enumerate(self.row_indices):
+                df.loc[idx] = [""] * df.shape[1]
+            self.model.endInsertRows()
+        else:
+            self.model.beginRemoveRows(QModelIndex(), min(self.row_indices), max(self.row_indices))
+            df.drop(index=self.old_ind_names, inplace=True)
+            self.model.endRemoveRows()
+
+    def undo(self):
+        df = self.model._data_frame
+
+        if self.add_mode:
+            positions = [df.index.get_loc(idx) for idx in self.row_indices]
+            self.model.beginRemoveRows(QModelIndex(), min(positions), max(positions))
+            df.drop(index=self.old_ind_names, inplace=True)
+            self.model.endRemoveRows()
+        else:
+            self.model.beginInsertRows(QModelIndex(), min(self.row_indices), max(self.row_indices))
+            restore_index_order = df.index
+            for pos, index_name, row in zip(
+                self.row_indices, self.old_ind_names, self.old_rows.values
+            ):
+                restore_index_order = restore_index_order.insert(
+                    pos, index_name
+                )
+                df.loc[index_name] = row
+                df.sort_index(
+                    inplace=True,
+                    key=lambda x: x.map(restore_index_order.get_loc)
+                )
+            self.model.endInsertRows()
+
+
+class ModifyDataFrameCommand(QUndoCommand):
+    def __init__(self, model, changes: dict[tuple, tuple], description="Modify values"):
+        super().__init__(description)
+        self.model = model
+        self.changes = changes  # {(row_key, column_name): (old_val, new_val)}
+
+    def redo(self):
+        self._apply_changes(use_new=True)
+
+    def undo(self):
+        self._apply_changes(use_new=False)
+
+    def _apply_changes(self, use_new: bool):
+        df = self.model._data_frame
+        col_offset = 1 if self.model._has_named_index else 0
+        original_dtypes = df.dtypes.copy()
+
+        # Apply changes
+        update_vals = {
+            (row, col): val[1 if use_new else 0]
+            for (row, col), val in self.changes.items()
+        }
+        update_df = pd.Series(update_vals).unstack()
+        for col in update_df.columns:
+            if col in df.columns:
+                df[col] = df[col].astype('object')
+        update_df.replace({None: "Placeholder_temp"}, inplace=True)
+        df.update(update_df)
+        df.replace({"Placeholder_temp": ""}, inplace=True)
+        for col, dtype in original_dtypes.items():
+            if col not in update_df.columns:
+                continue
+            if np.issubdtype(dtype, np.number):
+                df[col] = pd.to_numeric(df[col], errors="coerce")
+            else:
+                df[col] = df[col].astype(dtype)
+
+        rows = [df.index.get_loc(row_key) for (row_key, _) in
+                self.changes.keys()]
+        cols = [df.columns.get_loc(col) + col_offset for (_, col) in
+                self.changes.keys()]
+
+        top_left = self.model.index(min(rows), min(cols))
+        bottom_right = self.model.index(max(rows), max(cols))
+        self.model.dataChanged.emit(top_left, bottom_right, [Qt.DisplayRole])
+
+
+class RenameIndexCommand(QUndoCommand):
+    def __init__(self, model, old_index, new_index, model_index):
+        super().__init__(f"Rename index {old_index} → {new_index}")
+        self.model = model
+        self.model_index = model_index
+        self.old_index = old_index
+        self.new_index = new_index
+
+    def redo(self):
+        self._apply(self.old_index, self.new_index)
+
+    def undo(self):
+        self._apply(self.new_index, self.old_index)
+
+    def _apply(self, src, dst):
+        df = self.model._data_frame
+        df.rename(index={src: dst}, inplace=True)
+        self.model.dataChanged.emit(
+            self.model_index, self.model_index, [Qt.DisplayRole]
+        )

src/petab_gui/controllers/mother_controller.py

@@ -2,7 +2,7 @@
 
 from PySide6.QtWidgets import QMessageBox, QFileDialog, QLineEdit, QWidget, \
     QHBoxLayout, QToolButton, QTableView
-from PySide6.QtGui import QAction, QDesktopServices
+from PySide6.QtGui import QAction, QDesktopServices, QUndoStack, QKeySequence
 import zipfile
 import tempfile
 import os
@@ -19,7 +19,7 @@
     ConditionController, ParameterController
 from .logger_controller import LoggerController
 from ..views import TaskBar
-from .utils import prompt_overwrite_or_append, RecentFilesManager
+from .utils import prompt_overwrite_or_append, RecentFilesManager, filtered_error
 from functools import partial
 from ..settings_manager import SettingsDialog, settings_manager
 
@@ -42,6 +42,7 @@ def __init__(self, view, model: PEtabModel):
         model: PEtabModel
             The PEtab model.
         """
+        self.undo_stack = QUndoStack()
         self.task_bar = None
         self.view = view
         self.model = model
@@ -51,24 +52,28 @@ def __init__(self, view, model: PEtabModel):
             self.view.measurement_dock,
             self.model.measurement,
             self.logger,
+            self.undo_stack,
             self
         )
         self.observable_controller = ObservableController(
             self.view.observable_dock,
             self.model.observable,
             self.logger,
+            self.undo_stack,
             self
         )
         self.parameter_controller = ParameterController(
             self.view.parameter_dock,
             self.model.parameter,
             self.logger,
+            self.undo_stack,
             self
         )
         self.condition_controller = ConditionController(
             self.view.condition_dock,
             self.model.condition,
             self.logger,
+            self.undo_stack,
             self
         )
         self.sbml_controller = SbmlController(
@@ -189,21 +194,21 @@ def setup_actions(self):
             "&Close", self.view
         )}
         # Close
-        actions["close"].setShortcut("Ctrl+Q")
+        actions["close"].setShortcut(QKeySequence.Close)
         actions["close"].triggered.connect(self.view.close)
         # New File
         actions["new"] = QAction(
             qta.icon("mdi6.file-document"),
             "&New", self.view
         )
-        actions["new"].setShortcut("Ctrl+N")
+        actions["new"].setShortcut(QKeySequence.New)
         actions["new"].triggered.connect(self.new_file)
         # Open File
         actions["open"] = QAction(
             qta.icon("mdi6.folder-open"),
             "&Open", self.view
         )
-        actions["open"].setShortcut("Ctrl+O")
+        actions["open"].setShortcut(QKeySequence.Open)
         actions["open"].triggered.connect(
             partial(self.open_file, mode="overwrite")
         )
@@ -221,33 +226,33 @@ def setup_actions(self):
             qta.icon("mdi6.content-save-all"),
             "&Save", self.view
         )
-        actions["save"].setShortcut("Ctrl+S")
+        actions["save"].setShortcut(QKeySequence.Save)
         actions["save"].triggered.connect(self.save_model)
         # Find + Replace
         actions["find"] = QAction(
             qta.icon("mdi6.magnify"),
             "Find", self.view
         )
-        actions["find"].setShortcut("Ctrl+F")
+        actions["find"].setShortcut(QKeySequence.Find)
         actions["find"].triggered.connect(self.find)
         actions["find+replace"] = QAction(
             qta.icon("mdi6.find-replace"),
             "Find/Replace", self.view
         )
-        actions["find+replace"].setShortcut("Ctrl+R")
+        actions["find+replace"].setShortcut(QKeySequence.Replace)
         actions["find+replace"].triggered.connect(self.replace)
         # Copy / Paste
         actions["copy"] = QAction(
             qta.icon("mdi6.content-copy"),
             "Copy", self.view
         )
-        actions["copy"].setShortcut("Ctrl+C")
+        actions["copy"].setShortcut(QKeySequence.Copy)
         actions["copy"].triggered.connect(self.copy_to_clipboard)
         actions["paste"] = QAction(
             qta.icon("mdi6.content-paste"),
             "Paste", self.view
         )
-        actions["paste"].setShortcut("Ctrl+V")
+        actions["paste"].setShortcut(QKeySequence.Paste)
         actions["paste"].triggered.connect(self.paste_from_clipboard)
         # add/delete row
         actions["add_row"] = QAction(
@@ -369,6 +374,23 @@ def setup_actions(self):
             ))
         )
 
+        # Undo / Redo
+        actions["undo"] = QAction(
+            qta.icon("mdi6.undo"),
+            "&Undo", self.view
+        )
+        actions["undo"].setShortcut(QKeySequence.Undo)
+        actions["undo"].triggered.connect(self.undo_stack.undo)
+        actions["undo"].setEnabled(self.undo_stack.canUndo())
+        self.undo_stack.canUndoChanged.connect(actions["undo"].setEnabled)
+        actions["redo"] = QAction(
+            qta.icon("mdi6.redo"),
+            "&Redo", self.view
+        )
+        actions["redo"].setShortcut(QKeySequence.Redo)
+        actions["redo"].triggered.connect(self.undo_stack.redo)
+        actions["redo"].setEnabled(self.undo_stack.canRedo())
+        self.undo_stack.canRedoChanged.connect(actions["redo"].setEnabled)
         return actions
 
     def sync_visibility_with_actions(self):
@@ -673,6 +695,9 @@ def check_model(self):
                     model.reset_invalid_cells()
             else:
                 self.logger.log_message("Model is inconsistent.", color="red")
+        except Exception as e:
+            msg = f"PEtab linter failed at some point: {filtered_error(e)}"
+            self.logger.log_message(msg, color="red")
         finally:
             # Always remove the capture handler
             logger.removeHandler(capture_handler)