From 325da865843d883b87dd851fc3446ce205bf991c Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Tue, 11 Mar 2025 22:14:57 +0100 Subject: [PATCH 1/9] Very fist Undo Command working --- src/petab_gui/commands.py | 33 +++++++++++++++++++ .../controllers/mother_controller.py | 14 +++++++- .../controllers/table_controllers.py | 3 ++ src/petab_gui/models/pandas_table_model.py | 30 ++++++++++++----- src/petab_gui/views/task_bar.py | 4 +++ 5 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 src/petab_gui/commands.py diff --git a/src/petab_gui/commands.py b/src/petab_gui/commands.py new file mode 100644 index 0000000..83698f1 --- /dev/null +++ b/src/petab_gui/commands.py @@ -0,0 +1,33 @@ +"""Store commands for the do/undo functionality.""" +import copy +from PySide6.QtGui import QUndoCommand +from PySide6.QtCore import QModelIndex + + +class AddColumnCommand(QUndoCommand): + """Command to add a column to the table.""" + + def __init__(self, model, column_name): + super().__init__( + f"Add column {column_name} in table {model.table_type}" + ) + self.model = model + self.column_name = column_name + self.position = copy.deepcopy(self.model.get_df().shape[1]) + self.was_added = False + + def redo(self): + self.model.beginInsertColumns( + QModelIndex(), self.position, self.position + ) + self.model._data_frame[self.column_name] = "" + self.model.endInsertColumns() + self.was_added = True + + def undo(self): + self.model.beginRemoveColumns( + QModelIndex(), self.position, self.position + ) + self.model._data_frame.drop(columns=self.column_name, inplace=True) + self.model.endRemoveColumns() + self.was_added = False diff --git a/src/petab_gui/controllers/mother_controller.py b/src/petab_gui/controllers/mother_controller.py index 759f7bf..c058157 100644 --- a/src/petab_gui/controllers/mother_controller.py +++ b/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 +from PySide6.QtGui import QAction, QUndoStack import zipfile import tempfile import os @@ -41,6 +41,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 @@ -50,24 +51,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( @@ -361,6 +366,13 @@ def setup_actions(self): ) actions["clear_log"].triggered.connect(self.logger.clear_log) + # Undo / Redo + actions["undo"] = QAction( + qta.icon("mdi6.undo"), + "&Undo", self.view + ) + actions["undo"].setShortcut("Ctrl+Z") + actions["undo"].triggered.connect(self.undo_stack.undo) return actions def save_model(self): diff --git a/src/petab_gui/controllers/table_controllers.py b/src/petab_gui/controllers/table_controllers.py index a0a9960..212e090 100644 --- a/src/petab_gui/controllers/table_controllers.py +++ b/src/petab_gui/controllers/table_controllers.py @@ -24,6 +24,7 @@ def __init__( view: TableViewer, model: PandasTableModel, logger, + undo_stack, mother_controller ): """Initialize the table controller. @@ -46,6 +47,8 @@ def __init__( self.model.view = self.view.table_view self.proxy_model = PandasTableFilterProxy(model) self.logger = logger + self.undo_stack = undo_stack + self.model.undo_stack = undo_stack self.check_petab_lint_mode = True self.mother_controller = mother_controller self.view.table_view.setModel(self.proxy_model) diff --git a/src/petab_gui/models/pandas_table_model.py b/src/petab_gui/models/pandas_table_model.py index fb6134f..5e1e96b 100644 --- a/src/petab_gui/models/pandas_table_model.py +++ b/src/petab_gui/models/pandas_table_model.py @@ -6,6 +6,7 @@ from ..C import COLUMNS from ..utils import validate_value, create_empty_dataframe, is_invalid, \ get_selected +from ..commands import AddColumnCommand class PandasTableModel(QAbstractTableModel): @@ -17,7 +18,8 @@ class PandasTableModel(QAbstractTableModel): something_changed = Signal(bool) inserted_row = Signal(QModelIndex) - def __init__(self, data_frame, allowed_columns, table_type, parent=None): + def __init__(self, data_frame, allowed_columns, table_type, + undo_stack = None, parent=None): super().__init__(parent) self._allowed_columns = allowed_columns self.table_type = table_type @@ -31,6 +33,7 @@ def __init__(self, data_frame, allowed_columns, table_type, parent=None): # offset for row and column to get from the data_frame to the view self.row_index_offset = 0 self.column_offset = 0 + self.undo_stack = undo_stack def rowCount(self, parent=QModelIndex()): return self._data_frame.shape[0] + 1 # empty row at the end @@ -112,21 +115,30 @@ def insertColumn(self, column_name: str): Override insertColumn to always add the column at the right (end) of the table, and do so in-place on the DataFrame. """ + if column_name in self._data_frame.columns: + self.new_log_message.emit( + f"Column '{column_name}' already exists", + "red" + ) + return False if not ( column_name in self._allowed_columns or self.table_type == "condition" ): # empty dict means all columns allowed self.new_log_message.emit( - f"Column '{column_name}' not allowed in {self.table_type} table", + f"Column '{column_name}' will be ignored for the petab " + f"problem but may still be used to store relevant information", "orange" ) - position = self._data_frame.shape[1] - self.beginInsertColumns(QModelIndex(), position, position) - column_type = \ - self._allowed_columns.get(column_name, {"type": "STRING"})["type"] - default_value = "" if column_type == "STRING" else 0 - self._data_frame[column_name] = default_value - self.endInsertColumns() + + if self.undo_stack: + print(f"Before push: {self.undo_stack.count()}") # Debugging line + self.undo_stack.push(AddColumnCommand(self, column_name)) + print(f"After push: {self.undo_stack.count()}") + else: + # Fallback if undo stack isn't used + command = AddColumnCommand(self, column_name) + command.redo() return True diff --git a/src/petab_gui/views/task_bar.py b/src/petab_gui/views/task_bar.py index 97ad4c3..24ad262 100644 --- a/src/petab_gui/views/task_bar.py +++ b/src/petab_gui/views/task_bar.py @@ -67,9 +67,13 @@ def __init__(self, parent, actions): # Find and Replace self.find_replace_action = self.add_action_or_menu("Find/Replace") + self.menu.addSeparator() # Copy, Paste self.menu.addAction(actions["copy"]) self.menu.addAction(actions["paste"]) + self.menu.addSeparator() + # Undo, Redo + self.menu.addAction(actions["undo"]) # Add Columns self.menu.addAction(actions["add_column"]) self.menu.addAction(actions["delete_column"]) From b804daa86fc17e31ad34d9bc5e37cc8405beedd9 Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Tue, 11 Mar 2025 22:36:10 +0100 Subject: [PATCH 2/9] Completed column alteration --- src/petab_gui/commands.py | 56 ++++++++++++++----- .../controllers/table_controllers.py | 2 +- src/petab_gui/models/pandas_table_model.py | 17 +++--- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/petab_gui/commands.py b/src/petab_gui/commands.py index 83698f1..6e887e7 100644 --- a/src/petab_gui/commands.py +++ b/src/petab_gui/commands.py @@ -4,30 +4,56 @@ from PySide6.QtCore import QModelIndex -class AddColumnCommand(QUndoCommand): +class ModifyColumnCommand(QUndoCommand): """Command to add a column to the table.""" - def __init__(self, model, column_name): + def __init__(self, model, column_name, add_mode: bool = True): + action = "Add" if add_mode else "Remove" super().__init__( - f"Add column {column_name} in table {model.table_type}" + f"{action} column {column_name} in table {model.table_type}" ) self.model = model self.column_name = column_name - self.position = copy.deepcopy(self.model.get_df().shape[1]) - self.was_added = False + self.add_mode = add_mode + self.old_values = None + if not add_mode and column_name in model._data_frame.columns: + self.old_values = model._data_frame[column_name].copy() def redo(self): - self.model.beginInsertColumns( - QModelIndex(), self.position, self.position - ) - self.model._data_frame[self.column_name] = "" - self.model.endInsertColumns() - self.was_added = True + 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: + position = list( + self.model._data_frame.columns + ).index(self.column_name) + self.model.beginRemoveColumns( + QModelIndex(), position, position + ) + self.model._data_frame.drop(columns=self.column_name, inplace=True) + self.model.endRemoveColumns() def undo(self): + if self.add_mode: + position = list( + self.model._data_frame.columns + ).index(self.column_name) + self.model.beginRemoveColumns( + QModelIndex(), position, position + ) + self.model._data_frame.drop(columns=self.column_name, inplace=True) + self.model.endRemoveColumns() + else: + position = self.model._data_frame.shape[1] + self.model.beginInsertColumns( + QModelIndex(), position, position + ) + self.model._data_frame[self.column_name] = self.old_values + self.model.endInsertColumns() self.model.beginRemoveColumns( - QModelIndex(), self.position, self.position + QModelIndex(), position, position ) - self.model._data_frame.drop(columns=self.column_name, inplace=True) - self.model.endRemoveColumns() - self.was_added = False diff --git a/src/petab_gui/controllers/table_controllers.py b/src/petab_gui/controllers/table_controllers.py index 212e090..226138e 100644 --- a/src/petab_gui/controllers/table_controllers.py +++ b/src/petab_gui/controllers/table_controllers.py @@ -253,7 +253,7 @@ def delete_column(self): self.logger.log_message( f"Cannot delete column {column_name}, as it is a " f"required column!", - color = "red" + color="red" ) continue if column_name in self.completers: diff --git a/src/petab_gui/models/pandas_table_model.py b/src/petab_gui/models/pandas_table_model.py index 5e1e96b..c14f629 100644 --- a/src/petab_gui/models/pandas_table_model.py +++ b/src/petab_gui/models/pandas_table_model.py @@ -6,7 +6,7 @@ from ..C import COLUMNS from ..utils import validate_value, create_empty_dataframe, is_invalid, \ get_selected -from ..commands import AddColumnCommand +from ..commands import ModifyColumnCommand class PandasTableModel(QAbstractTableModel): @@ -132,12 +132,10 @@ def insertColumn(self, column_name: str): ) if self.undo_stack: - print(f"Before push: {self.undo_stack.count()}") # Debugging line - self.undo_stack.push(AddColumnCommand(self, column_name)) - print(f"After push: {self.undo_stack.count()}") + self.undo_stack.push(ModifyColumnCommand(self, column_name)) else: # Fallback if undo stack isn't used - command = AddColumnCommand(self, column_name) + command = ModifyColumnCommand(self, column_name) command.redo() return True @@ -349,9 +347,12 @@ def delete_row(self, row): def delete_column(self, column_index): """Delete a column from the DataFrame.""" column_name = self._data_frame.columns[column_index - self.column_offset] - self.beginRemoveColumns(QModelIndex(), column_index, column_index) - self._data_frame.drop(columns=[column_name], inplace=True) - self.endRemoveColumns() + if self.undo_stack: + self.undo_stack.push(ModifyColumnCommand(self, column_name, False)) + else: + # Fallback if undo stack isn't used + command = ModifyColumnCommand(self, column_name, False) + command.redo() def clear_table(self): """Clear the table.""" From c9d147a371ea1aa0700f0be2ad456d0c031f0da5 Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Wed, 16 Apr 2025 15:39:25 +0200 Subject: [PATCH 3/9] More stable undo redo of a singular column --- src/petab_gui/commands.py | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/petab_gui/commands.py b/src/petab_gui/commands.py index 6e887e7..c28f3f5 100644 --- a/src/petab_gui/commands.py +++ b/src/petab_gui/commands.py @@ -16,44 +16,31 @@ def __init__(self, model, column_name, add_mode: bool = True): 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.beginInsertColumns(QModelIndex(), position, position) self.model._data_frame[self.column_name] = "" self.model.endInsertColumns() else: - position = list( - self.model._data_frame.columns - ).index(self.column_name) - self.model.beginRemoveColumns( - QModelIndex(), position, position - ) + 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 = list( - self.model._data_frame.columns - ).index(self.column_name) - self.model.beginRemoveColumns( - QModelIndex(), position, position - ) + 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: - position = self.model._data_frame.shape[1] - self.model.beginInsertColumns( - QModelIndex(), position, position - ) - self.model._data_frame[self.column_name] = self.old_values + self.model.beginInsertColumns(QModelIndex(), self.position, self.position) + self.model._data_frame.insert(self.position, self.column_name, self.old_values) self.model.endInsertColumns() - self.model.beginRemoveColumns( - QModelIndex(), position, position - ) From f842d052c81ebcbc33713944458c3311ad050563 Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Thu, 17 Apr 2025 15:14:37 +0200 Subject: [PATCH 4/9] adding and deleting rows is now reversible --- src/petab_gui/commands.py | 82 +++++++++++++++++++++- src/petab_gui/models/pandas_table_model.py | 28 ++++---- 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/src/petab_gui/commands.py b/src/petab_gui/commands.py index c28f3f5..90a8d43 100644 --- a/src/petab_gui/commands.py +++ b/src/petab_gui/commands.py @@ -1,7 +1,7 @@ """Store commands for the do/undo functionality.""" -import copy from PySide6.QtGui import QUndoCommand from PySide6.QtCore import QModelIndex +import numpy as np class ModifyColumnCommand(QUndoCommand): @@ -44,3 +44,83 @@ def undo(self): 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 + 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() diff --git a/src/petab_gui/models/pandas_table_model.py b/src/petab_gui/models/pandas_table_model.py index 6ddf8c1..30ef7a1 100644 --- a/src/petab_gui/models/pandas_table_model.py +++ b/src/petab_gui/models/pandas_table_model.py @@ -8,7 +8,7 @@ get_selected from ..controllers.default_handler import DefaultHandlerModel from ..settings_manager import settings_manager -from ..commands import ModifyColumnCommand +from ..commands import ModifyColumnCommand, ModifyRowCommand class PandasTableModel(QAbstractTableModel): @@ -109,17 +109,12 @@ def insertRows(self, position, rows, parent=QModelIndex()) -> bool: -------- bool: True if rows were added successfully. """ - end_position = len(self._data_frame) - self.beginInsertRows( - QModelIndex(), end_position, end_position + rows - 1 - ) - - # In-place row addition using loc - for i in range(rows): - # Append an empty row or row with default values using loc - self._data_frame.loc[end_position + i] = \ - [""] * self._data_frame.shape[1] - self.endInsertRows() + if self.undo_stack: + self.undo_stack.push(ModifyRowCommand(self, rows)) + else: + # Fallback if undo stack isn't used + command = ModifyRowCommand(self, rows) + command.redo() return True def insertColumn(self, column_name: str): @@ -373,9 +368,12 @@ def unique_values(self, column_name): def delete_row(self, row): """Delete a row from the table.""" - self.beginRemoveRows(QModelIndex(), row, row) - self._data_frame.drop(self._data_frame.index[row], inplace=True) - self.endRemoveRows() + if self.undo_stack: + self.undo_stack.push(ModifyRowCommand(self, row, False)) + else: + # Fallback if undo stack isn't used + command = ModifyRowCommand(self, row, False) + command.redo() def delete_column(self, column_index): """Delete a column from the DataFrame.""" From f79423183c6f8843a4e41d7a131e1086625e10fc Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Wed, 23 Apr 2025 09:35:10 +0200 Subject: [PATCH 5/9] Undo Redo provisourically complete, testing remains --- src/petab_gui/commands.py | 63 ++++- .../controllers/mother_controller.py | 7 + .../controllers/table_controllers.py | 10 +- src/petab_gui/models/pandas_table_model.py | 235 ++++++++++-------- src/petab_gui/models/petab_model.py | 1 - src/petab_gui/utils.py | 75 +++--- src/petab_gui/views/task_bar.py | 1 + 7 files changed, 242 insertions(+), 150 deletions(-) diff --git a/src/petab_gui/commands.py b/src/petab_gui/commands.py index 90a8d43..90dc7f9 100644 --- a/src/petab_gui/commands.py +++ b/src/petab_gui/commands.py @@ -1,7 +1,7 @@ """Store commands for the do/undo functionality.""" from PySide6.QtGui import QUndoCommand -from PySide6.QtCore import QModelIndex -import numpy as np +from PySide6.QtCore import QModelIndex, Qt +import pandas as pd class ModifyColumnCommand(QUndoCommand): @@ -85,6 +85,7 @@ def _generate_new_indices(self, count): if idx not in existing: indices.append(idx) base += 1 + self.old_ind_names = indices return indices def redo(self): @@ -124,3 +125,61 @@ def undo(self): 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 + + # 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() + update_df.replace({None: "Placeholder_temp"}, inplace=True) + df.update(update_df) + df.replace({"Placeholder_temp": ""}, inplace=True) + + 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] + ) diff --git a/src/petab_gui/controllers/mother_controller.py b/src/petab_gui/controllers/mother_controller.py index bdcafdf..3d8457f 100644 --- a/src/petab_gui/controllers/mother_controller.py +++ b/src/petab_gui/controllers/mother_controller.py @@ -381,6 +381,13 @@ def setup_actions(self): ) actions["undo"].setShortcut("Ctrl+Z") actions["undo"].triggered.connect(self.undo_stack.undo) + actions["redo"] = QAction( + qta.icon("mdi6.redo"), + "&Redo", self.view + ) + # set shortcut as ctrl + z + Arrow up + actions["redo"].setShortcut("Ctrl+Shift+Z") + actions["redo"].triggered.connect(self.undo_stack.redo) return actions def sync_visibility_with_actions(self): diff --git a/src/petab_gui/controllers/table_controllers.py b/src/petab_gui/controllers/table_controllers.py index a0e3774..a5d9e6f 100644 --- a/src/petab_gui/controllers/table_controllers.py +++ b/src/petab_gui/controllers/table_controllers.py @@ -11,8 +11,8 @@ from ..settings_manager import settings_manager from ..views.table_view import TableViewer, SingleSuggestionDelegate, \ ColumnSuggestionDelegate, ComboBoxDelegate, ParameterIdSuggestionDelegate -from ..utils import get_selected, process_file -from .utils import prompt_overwrite_or_append +from ..utils import get_selected, process_file, ConditionInputDialog +from .utils import prompt_overwrite_or_append, linter_wrapper from ..C import COLUMN import re @@ -598,9 +598,9 @@ def process_data_matrix_file(self, file_name, mode, separator=None): cond_dialog = ConditionInputDialog() if cond_dialog.exec(): - conditions = cond_dialog.get_condition_id() - condition_id = conditions.get("conditionId", "") - preeq_id = conditions.get("preeq_id", "") + conditions = cond_dialog.get_inputs() + condition_id = conditions.get("simulationConditionId", "") + preeq_id = conditions.get("preequilibrationConditionId", "") if mode == "overwrite": self.model.clear_table() self.populate_tables_from_data_matrix( diff --git a/src/petab_gui/models/pandas_table_model.py b/src/petab_gui/models/pandas_table_model.py index 30ef7a1..305513b 100644 --- a/src/petab_gui/models/pandas_table_model.py +++ b/src/petab_gui/models/pandas_table_model.py @@ -8,7 +8,8 @@ get_selected from ..controllers.default_handler import DefaultHandlerModel from ..settings_manager import settings_manager -from ..commands import ModifyColumnCommand, ModifyRowCommand +from ..commands import (ModifyColumnCommand, ModifyRowCommand, + ModifyDataFrameCommand, RenameIndexCommand) class PandasTableModel(QAbstractTableModel): @@ -159,11 +160,16 @@ def setData(self, index, value, role=Qt.EditRole): # check whether multiple rows but only one column is selected multi_row_change, selected = self.check_selection() if not multi_row_change: - return self._set_data_single(index, value) + self.undo_stack.beginMacro("Set data") + success = self._set_data_single(index, value) + self.undo_stack.endMacro() + return success # multiple rows but only one column is selected all_set = list() + self.undo_stack.beginMacro("Set data") for index in selected: all_set.append(self._set_data_single(index, value)) + self.undo_stack.endMacro() return all(all_set) def _set_data_single(self, index, value): @@ -175,7 +181,6 @@ def _set_data_single(self, index, value): # empty row at the end self.insertRows(index.row(), 1) self.fill_defaults.emit(index) - # self.get_default_values(index) next_index = self.index(index.row(), 0) self.inserted_row.emit(next_index) if index.column() == 0 and self._has_named_index: @@ -187,9 +192,10 @@ def _set_data_single(self, index, value): # cast to numeric if necessary expected_type = self._allowed_columns.get(column_name, None) if is_invalid(value): - if not expected_type["optional"]: - return False - self._data_frame.iloc[row, column - col_setoff] = None + change = { + (self._data_frame.index[row], column_name): (old_value, None)} + self.undo_stack.push(ModifyDataFrameCommand(self, change)) + self.cell_needs_validation.emit(row, column) self.dataChanged.emit(index, index, [Qt.DisplayRole]) return True if expected_type: @@ -205,7 +211,9 @@ def _set_data_single(self, index, value): return False if column_name == "observableId": - self._data_frame.iloc[row, column - col_setoff] = value + change = { + (self._data_frame.index[row], column_name): (old_value, value)} + self.undo_stack.push(ModifyDataFrameCommand(self, change)) self.dataChanged.emit(index, index, [Qt.DisplayRole]) self.relevant_id_changed.emit(value, old_value, "observable") self.cell_needs_validation.emit(row, column) @@ -213,7 +221,9 @@ def _set_data_single(self, index, value): return True if column_name in ["conditionId", "simulationConditionId", "preequilibrationConditionId"]: - self._data_frame.iloc[row, column - col_setoff] = value + change = { + (self._data_frame.index[row], column_name): (old_value, value)} + self.undo_stack.push(ModifyDataFrameCommand(self, change)) self.dataChanged.emit(index, index, [Qt.DisplayRole]) self.relevant_id_changed.emit(value, old_value, "condition") self.cell_needs_validation.emit(row, column) @@ -236,7 +246,9 @@ def _set_data_single(self, index, value): ) return False # Set the new value - self._data_frame.iloc[row, column - col_setoff] = value + change = { + (self._data_frame.index[row], column_name): (old_value, value)} + self.undo_stack.push(ModifyDataFrameCommand(self, change)) # Validate the row after setting data self.cell_needs_validation.emit(row, column) self.something_changed.emit(True) @@ -500,6 +512,47 @@ def endResetModel(self): self.config = settings_manager.get_table_defaults(self.table_type) self.default_handler = DefaultHandlerModel(self, self.config) + def fill_row(self, row_position: int, data: dict): + """Fill a row with data. + + Parameters + ---------- + row_position: + The position of the row to fill. + data: + The data to fill the row with. Gets updated with default values. + """ + # TODO: Needs undo command, needs syncing with other commands + # TODO: When undoing a lne and adding then meas -> add both lines + data_to_add = { + column_name: "" for column_name in self._data_frame.columns + } + unknown_keys = set(data) - set(self._data_frame.columns) + for key in unknown_keys: + if key == self._data_frame.index.name: + continue + data.pop(key, None) + data_to_add.update(data) + index_key = None + if self.table_type == "condition": + index_key = data_to_add.pop("conditionId") + elif self.table_type == "observable": + index_key = data_to_add.pop("observableId") + if index_key and self._has_named_index: + self.undo_stack.push(RenameIndexCommand( + self, self._data_frame.index.tolist()[row_position], + index_key, self.index(row_position, 0) + )) + + changes = { + (index_key, col): (self._data_frame.at[index_key, col], val) + for col, val in data_to_add.items() + } + self.undo_stack.push(ModifyDataFrameCommand( + self, changes, "Fill values" + )) + self.fill_defaults.emit(self.index(row_position, 0)) + class IndexedPandasTableModel(PandasTableModel): """Table model for tables with named index.""" @@ -517,30 +570,54 @@ def __init__(self, data_frame, allowed_columns, table_type, parent=None): def get_default_values(self, index): """Return the default values for a the row in a new index.""" - row = index.row() - if isinstance(row, int): - row = self._data_frame.index[row] - columns_with_index = ( - [self._data_frame.index.name or "index"] + - list(self._data_frame.columns) - ) + row_idx = index.row() + df = self._data_frame + if isinstance(row_idx, int): + row_key = df.index[row_idx] + else: + row_key = row_idx + changes = {} + rename_needed = False + old_index = row_key + new_index = row_key + + columns_with_index = [df.index.name] if df.index.name else [] + columns_with_index += list(df.columns) + for colname in columns_with_index: - if colname == self._data_frame.index.name and not isinstance(row, int): - continue - if colname == self._data_frame.index.name and isinstance(row, int): - default_value = self.default_handler.get_default(colname, row) - if default_value == "": - default_value = f"{self.table_type}_{row}" - self._data_frame.rename( - index={self._data_frame.index[row]: default_value}, - inplace=True - ) - row = default_value # Update row to new index - continue - # if column is empty, fill with default value - if self._data_frame.loc[row, colname] == "": - default_value = self.default_handler.get_default(colname, row) - self._data_frame.loc[row, colname] = default_value + if colname == df.index.name: + # Generate default index name if empty + default_value = self.default_handler.get_default(colname, row_key) + if ( + not row_key + or f"new_{self.table_type}" in row_key + ): + rename_needed = True + new_index = default_value + else: + if df.at[row_key, colname] == "": + default_value = self.default_handler.get_default(colname, row_key) + changes[(row_key, colname)] = ("", default_value) + + commands = [] + if changes: + commands.append(ModifyDataFrameCommand( + self, changes, "Fill default values" + )) + if rename_needed: + commands.append(RenameIndexCommand( + self, old_index, new_index, index + )) + if not commands: + return + if not self.undo_stack: + for command in commands: + command.redo() + return + self.undo_stack.beginMacro("Fill default values") + for command in commands: + self.undo_stack.push(command) + self.undo_stack.endMacro() def handle_named_index(self, index, value): """Handle the named index column.""" @@ -555,8 +632,9 @@ def handle_named_index(self, index, value): ) return False try: - self._data_frame.rename(index={old_value: value}, inplace=True) - self.dataChanged.emit(index, index, [Qt.DisplayRole]) + self.undo_stack.push(RenameIndexCommand( + self, old_value, value, index + )) self.relevant_id_changed.emit(value, old_value, self.table_type) self.cell_needs_validation.emit(row, 0) self.something_changed.emit(True) @@ -593,16 +671,24 @@ def __init__(self, data_frame, parent=None): def get_default_values(self, index): """Fill missing values in a row without modifying the index.""" row = index.row() + df = self._data_frame if isinstance(row, int): row_key = self._data_frame.index[row] else: row_key = row - for colname in self._data_frame.columns: - if self._data_frame.at[row_key, colname] == "": - default_value = self.default_handler.get_default(colname, - row_key) - self._data_frame.at[row_key, colname] = default_value + changes = {} + for colname in df.columns: + if df.at[row_key, colname] == "": + default = self.default_handler.get_default(colname, row_key) + changes[(row_key, colname)] = ("", default) + command = ModifyDataFrameCommand( + self, changes, "Fill default values" + ) + if self.undo_stack: + self.undo_stack.push(command) + else: + command.redo() def data(self, index, role=Qt.DisplayRole): """Return the data at the given index and role for the View.""" @@ -638,26 +724,6 @@ def headerData(self, section, orientation, role=Qt.DisplayRole): return str(section) return None - def fill_row(self, row_position: int, data: dict): - """Fill a row with data. - - Parameters - ---------- - row_position: - The position of the row to fill. - data: - The data to fill the row with. Gets updated with default values. - """ - data_to_add = { - column_name: "" for column_name in self._data_frame.columns - } - # remove preequilibrationConditionId if not in columns - if "preequilibrationConditionId" not in self._data_frame.columns: - data.pop("preequilibrationConditionId", None) - data_to_add.update(data) - # Maybe add default values for missing columns - self._data_frame.iloc[row_position] = data_to_add - def return_column_index(self, column_name): """Return the index of a column.""" if column_name in self._data_frame.columns: @@ -676,32 +742,6 @@ def __init__(self, data_frame, parent=None): parent=parent ) - def fill_row(self, row_position: int, data: dict): - """Fill a row with data. - - Parameters - ---------- - row_position: - The position of the row to fill. - data: - The data to fill the row with. Gets updated with default values. - """ - data_to_add = { - column_name: "" for column_name in self._data_frame.columns - } - data_to_add.update(data) - # Maybe add default values for missing columns? - new_index = self._data_frame.index.tolist() - index_name = self._data_frame.index.name - new_index[row_position] = data_to_add.pop( - "observableId" - ) - self._data_frame.index = pd.Index(new_index, name=index_name) - self._data_frame.iloc[row_position] = data_to_add - # make a QModelIndex for the new row - new_index = self.index(row_position, 0) - self.fill_defaults.emit(new_index) - class ParameterModel(IndexedPandasTableModel): """Table model for the parameter data.""" @@ -727,31 +767,6 @@ def __init__(self, data_frame, parent=None): ) self._allowed_columns.pop("conditionId") - def fill_row(self, row_position: int, data: dict): - """Fill a row with data. - - Parameters - ---------- - row_position: - The position of the row to fill. - data: - The data to fill the row with. Gets updated with default values. - """ - data_to_add = { - column_name: "" for column_name in self._data_frame.columns - } - data_to_add.update(data) - new_index = self._data_frame.index.tolist() - index_name = self._data_frame.index.name - new_index[row_position] = data_to_add.pop( - "conditionId" - ) - self._data_frame.index = pd.Index(new_index, name=index_name) - self._data_frame.iloc[row_position] = data_to_add - # make a QModelIndex for the new row - new_index = self.index(row_position, 0) - self.fill_defaults.emit(new_index) - class PandasTableFilterProxy(QSortFilterProxyModel): def __init__(self, model, parent=None): diff --git a/src/petab_gui/models/petab_model.py b/src/petab_gui/models/petab_model.py index d292fcb..be0509d 100644 --- a/src/petab_gui/models/petab_model.py +++ b/src/petab_gui/models/petab_model.py @@ -108,7 +108,6 @@ def test_consistency(self) -> bool: bool Whether the data is consistent. """ - # TODO: Create logging messages return petab.lint.lint_problem(self.current_petab_problem) def save(self, directory: str | Path): diff --git a/src/petab_gui/utils.py b/src/petab_gui/utils.py index 279bb16..0c79122 100644 --- a/src/petab_gui/utils.py +++ b/src/petab_gui/utils.py @@ -74,37 +74,38 @@ def antimonyToSBML(ant): class ConditionInputDialog(QDialog): - def __init__(self, condition_id, condition_columns, initial_values=None, error_key=None, parent=None): + def __init__(self, condition_id=None, parent=None): super().__init__(parent) self.setWindowTitle("Add Condition") self.layout = QVBoxLayout(self) - - # Condition ID - self.condition_id_layout = QHBoxLayout() - self.condition_id_label = QLabel("Condition ID:", self) - self.condition_id_input = QLineEdit(self) - self.condition_id_input.setText(condition_id) - self.condition_id_input.setReadOnly(True) - self.condition_id_layout.addWidget(self.condition_id_label) - self.condition_id_layout.addWidget(self.condition_id_input) - self.layout.addLayout(self.condition_id_layout) - - # Dynamic fields for existing columns - self.fields = {} - for column in condition_columns: - if column != "conditionId": # Skip conditionId - field_layout = QHBoxLayout() - field_label = QLabel(f"{column}:", self) - field_input = QLineEdit(self) - if initial_values and column in initial_values: - field_input.setText(str(initial_values[column])) - if column == error_key: - field_input.setStyleSheet("background-color: red;") - field_layout.addWidget(field_label) - field_layout.addWidget(field_input) - self.layout.addLayout(field_layout) - self.fields[column] = field_input + self.notification_label = QLabel("", self) + self.notification_label.setStyleSheet("color: red;") + self.notification_label.setVisible(False) + self.layout.addWidget(self.notification_label) + + # Simulation Condition + sim_layout = QHBoxLayout() + sim_label = QLabel("Simulation Condition:", self) + self.sim_input = QLineEdit(self) + if condition_id: + self.sim_input.setText(condition_id) + sim_layout.addWidget(sim_label) + sim_layout.addWidget(self.sim_input) + self.layout.addLayout(sim_layout) + + # Preequilibration Condition + preeq_layout = QHBoxLayout() + preeq_label = QLabel("Preequilibration Condition:", self) + self.preeq_input = QLineEdit(self) + self.preeq_input.setToolTip( + "This field is only needed when your experiment started in steady " + "state. In this case add here the experimental condition id for " + "the steady state." + ) + preeq_layout.addWidget(preeq_label) + preeq_layout.addWidget(self.preeq_input) + self.layout.addLayout(preeq_layout) # Buttons self.buttons_layout = QHBoxLayout() @@ -117,10 +118,22 @@ def __init__(self, condition_id, condition_columns, initial_values=None, error_k self.ok_button.clicked.connect(self.accept) self.cancel_button.clicked.connect(self.reject) + def accept(self): + if not self.sim_input.text().strip(): + self.sim_input.setStyleSheet("background-color: red;") + self.notification_label.setText("Simulation Condition is required.") + self.notification_label.setVisible(True) + return + self.notification_label.setVisible(False) + self.sim_input.setStyleSheet("") + super().accept() + def get_inputs(self): - inputs = {column: field.text() for column, field in self.fields.items()} - inputs["conditionId"] = self.condition_id_input.text() - inputs["conditionName"] = inputs["conditionId"] + inputs = {} + inputs["simulationConditionId"] = self.sim_input.text() + preeq = self.preeq_input.text() + if preeq: + inputs["preequilibrationConditionId"] = preeq return inputs @@ -464,8 +477,6 @@ def replace(self): self.parent().sbml_viewer.antimony_text_edit.setPlainText(antimony_text) - - class SyntaxHighlighter(QSyntaxHighlighter): def __init__(self, parent=None): super().__init__(parent) diff --git a/src/petab_gui/views/task_bar.py b/src/petab_gui/views/task_bar.py index bcb004b..81a8226 100644 --- a/src/petab_gui/views/task_bar.py +++ b/src/petab_gui/views/task_bar.py @@ -75,6 +75,7 @@ def __init__(self, parent, actions): self.menu.addSeparator() # Undo, Redo self.menu.addAction(actions["undo"]) + self.menu.addAction(actions["redo"]) # Add Columns self.menu.addAction(actions["add_column"]) self.menu.addAction(actions["delete_column"]) From fb39f8d526041fca3ee409ad9627826a954fc1b1 Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Wed, 23 Apr 2025 09:35:48 +0200 Subject: [PATCH 6/9] More unified petab linting --- src/petab_gui/C.py | 5 +++ .../controllers/table_controllers.py | 44 +++++++++---------- src/petab_gui/controllers/utils.py | 44 +++++++++++++++++++ 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/src/petab_gui/C.py b/src/petab_gui/C.py index 814c95c..e7acf26 100644 --- a/src/petab_gui/C.py +++ b/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 '' " + r"expecting \{[^}]+\}" : "Invalid empty cell!" +} diff --git a/src/petab_gui/controllers/table_controllers.py b/src/petab_gui/controllers/table_controllers.py index a5d9e6f..a94ba5f 100644 --- a/src/petab_gui/controllers/table_controllers.py +++ b/src/petab_gui/controllers/table_controllers.py @@ -105,27 +105,19 @@ def validate_changed_cell(self, row, column): """Validate the changed cell and whether its linting is correct.""" if not self.check_petab_lint_mode: return - row_data = self.model.get_df().iloc[row] - index_name = self.model.get_df().index.name + df = self.model.get_df() + row_data = df.iloc[row] + index_name = df.index.name row_data = row_data.to_frame().T row_data.index.name = index_name - try: - self.check_petab_lint(row_data) + row_name = row_data.index[0] + col_name = df.columns[column] + is_valid = self.check_petab_lint(row_data, row_name, col_name) + if is_valid: for col in range(self.model.columnCount()): self.model.discard_invalid_cell(row, col) - error_message = None - except Exception as e: - error_message = str(e) - self.logger.log_message( - f"PEtab linter failed at row {row}, column {column}: " - f"{error_message}", - color="red" - ) - # Update invalid cells based on the error state - if error_message: - self.model.add_invalid_cell(row, column) else: - self.model.discard_invalid_cell(row, column) + self.model.add_invalid_cell(row, column) self.model.notify_data_color_change(row, column) def open_table(self, file_path=None, separator=None, mode="overwrite"): @@ -325,7 +317,12 @@ def paste_from_clipboard(self): color="red" ) - def check_petab_lint(self, row_data): + def check_petab_lint( + self, + row_data: pd.DataFrame = None, + row_name: str = None, + col_name: str = None + ): """Check a single row of the model with petablint.""" raise NotImplementedError( "This method must be implemented in child classes." @@ -486,11 +483,11 @@ def update_defaults(self, settings_changed): class MeasurementController(TableController): """Controller of the Measurement table.""" - def check_petab_lint(self, row_data: pd.DataFrame = None): + @linter_wrapper + def check_petab_lint(self, row_data: pd.DataFrame = None, row_name: str = None, col_name: str = None): """Check a number of rows of the model with petablint.""" if row_data is None: row_data = self.model.get_df() - # Can this be done more elegantly? observable_df = self.mother_controller.model.observable.get_df() return petab.check_measurement_df( row_data, @@ -759,7 +756,8 @@ def setup_connections_specific(self): self.update_handler_model ) - def check_petab_lint(self, row_data: pd.DataFrame = None): + @linter_wrapper + def check_petab_lint(self, row_data: pd.DataFrame = None, row_name: str = None, col_name: str = None): """Check a number of rows of the model with petablint.""" if row_data is None: row_data = self.model.get_df() @@ -907,7 +905,8 @@ def setup_connections_specific(self): self.update_handler_model ) - def check_petab_lint(self, row_data: pd.DataFrame = None): + @linter_wrapper + def check_petab_lint(self, row_data: pd.DataFrame = None, row_name: str = None, col_name: str = None): """Check a number of rows of the model with petablint.""" if row_data is None: row_data = self.model.get_df() @@ -1038,7 +1037,8 @@ def setup_completers(self): self.completers["parameterId"] ) - def check_petab_lint(self, row_data: pd.DataFrame = None): + @linter_wrapper + def check_petab_lint(self, row_data: pd.DataFrame = None, row_name: str = None, col_name: str = None): """Check a number of rows of the model with petablint.""" if row_data is None: row_data = self.model.get_df() diff --git a/src/petab_gui/controllers/utils.py b/src/petab_gui/controllers/utils.py index 5802b36..fb543f4 100644 --- a/src/petab_gui/controllers/utils.py +++ b/src/petab_gui/controllers/utils.py @@ -3,8 +3,52 @@ from PySide6.QtGui import QAction from collections import Counter from pathlib import Path +import functools +import pandas as pd +import re from ..settings_manager import settings_manager +from ..C import COMMON_ERRORS + + +def linter_wrapper(_func=None): + def decorator(func): + @functools.wraps(func) + def wrapper(self, row_data: pd.DataFrame = None, row_name: + str = None, col_name: str = None, *args, **kwargs): + try: + result = func( + self, row_data, row_name, col_name, *args, **kwargs + ) + return True + except Exception as e: + if row_name is not None and col_name is not None: + msg = f"PEtab linter failed at ({row_name}, {col_name}): {filtered_error(e)}" + else: + msg = f"PEtab linter failed: {filtered_error(e)}" + + self.logger.log_message(msg, color="red") + return False + return wrapper + if callable(_func): # used without parentheses + return decorator(_func) + return decorator + + +def filtered_error(error_message: BaseException) -> str: + """Filters know error message and reformulates them.""" + all_errors = "|".join( + f"(?P{pattern})" for i, pattern in enumerate(COMMON_ERRORS) + ) + regex = re.compile(all_errors) + replacement_values = list(COMMON_ERRORS.values()) + # Replace function + def replacer(match): + for i, _ in enumerate(COMMON_ERRORS): + if match.group(f"key{i}"): + return replacement_values[i] + return match.group(0) + return regex.sub(replacer, str(error_message)) def prompt_overwrite_or_append(controller): From a18938bfd6c88d5b882f6e49c69160a9fb262b2e Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Wed, 23 Apr 2025 10:30:58 +0200 Subject: [PATCH 7/9] Casting into correct datatypes now. Slight changes to error messages --- src/petab_gui/commands.py | 15 +++++++++++++++ src/petab_gui/controllers/mother_controller.py | 5 ++++- src/petab_gui/controllers/table_controllers.py | 5 ++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/petab_gui/commands.py b/src/petab_gui/commands.py index 90dc7f9..34336d2 100644 --- a/src/petab_gui/commands.py +++ b/src/petab_gui/commands.py @@ -2,6 +2,10 @@ 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): @@ -142,6 +146,7 @@ def undo(self): 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 = { @@ -149,9 +154,19 @@ def _apply_changes(self, use_new: bool): 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()] diff --git a/src/petab_gui/controllers/mother_controller.py b/src/petab_gui/controllers/mother_controller.py index 3d8457f..e68ffde 100644 --- a/src/petab_gui/controllers/mother_controller.py +++ b/src/petab_gui/controllers/mother_controller.py @@ -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 @@ -692,6 +692,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) diff --git a/src/petab_gui/controllers/table_controllers.py b/src/petab_gui/controllers/table_controllers.py index a94ba5f..7ae394f 100644 --- a/src/petab_gui/controllers/table_controllers.py +++ b/src/petab_gui/controllers/table_controllers.py @@ -111,7 +111,10 @@ def validate_changed_cell(self, row, column): row_data = row_data.to_frame().T row_data.index.name = index_name row_name = row_data.index[0] - col_name = df.columns[column] + if column == 0 and self.model._has_named_index: + col_name = index_name + else: + col_name = df.columns[column - self.model.column_offset] is_valid = self.check_petab_lint(row_data, row_name, col_name) if is_valid: for col in range(self.model.columnCount()): From 3cc7c9bfc10435de1e053b5ba402991a0a9583bb Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Wed, 23 Apr 2025 12:53:09 +0200 Subject: [PATCH 8/9] 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 --- .../controllers/table_controllers.py | 8 +- src/petab_gui/controllers/utils.py | 23 ++- src/petab_gui/models/pandas_table_model.py | 137 +++++++++--------- 3 files changed, 91 insertions(+), 77 deletions(-) diff --git a/src/petab_gui/controllers/table_controllers.py b/src/petab_gui/controllers/table_controllers.py index 7ae394f..7e6e964 100644 --- a/src/petab_gui/controllers/table_controllers.py +++ b/src/petab_gui/controllers/table_controllers.py @@ -89,9 +89,6 @@ def setup_connections(self): self.model.inserted_row.connect( self.set_index_on_new_row ) - self.model.fill_defaults.connect( - self.model.get_default_values, Qt.QueuedConnection - ) settings_manager.settings_changed.connect( self.update_defaults ) @@ -291,7 +288,8 @@ def add_column(self, column_name: str = None): def set_index_on_new_row(self, index: QModelIndex): """Set the index of the model when a new row is added.""" - self.view.table_view.setCurrentIndex(index) + proxy_index = self.proxy_model.mapFromSource(index) + self.view.table_view.setCurrentIndex(proxy_index) def filter_table(self, text): """Filter the table.""" @@ -1040,7 +1038,7 @@ def setup_completers(self): self.completers["parameterId"] ) - @linter_wrapper + @linter_wrapper(additional_error_check=True) def check_petab_lint(self, row_data: pd.DataFrame = None, row_name: str = None, col_name: str = None): """Check a number of rows of the model with petablint.""" if row_data is None: diff --git a/src/petab_gui/controllers/utils.py b/src/petab_gui/controllers/utils.py index fb543f4..7669022 100644 --- a/src/petab_gui/controllers/utils.py +++ b/src/petab_gui/controllers/utils.py @@ -11,7 +11,7 @@ from ..C import COMMON_ERRORS -def linter_wrapper(_func=None): +def linter_wrapper(_func=None, additional_error_check: bool = False): def decorator(func): @functools.wraps(func) def wrapper(self, row_data: pd.DataFrame = None, row_name: @@ -22,10 +22,27 @@ def wrapper(self, row_data: pd.DataFrame = None, row_name: ) return True except Exception as e: + err_msg = filtered_error(e) + if additional_error_check: + if "Missing parameter(s)" in err_msg: + match = re.search(r"\{(.+?)\}", err_msg) + missing_params = { + s.strip(" '") for s in match.group(1).split(",") + } + remain = { + p for p in missing_params + if p not in self.model._data_frame.index + } + if not remain: + return True + err_msg = re.sub( + r"\{.*?\}", "{" + ", ".join(sorted(remain)) + "}", + err_msg + ) if row_name is not None and col_name is not None: - msg = f"PEtab linter failed at ({row_name}, {col_name}): {filtered_error(e)}" + msg = f"PEtab linter failed at ({row_name}, {col_name}): {err_msg}" else: - msg = f"PEtab linter failed: {filtered_error(e)}" + msg = f"PEtab linter failed: {err_msg}" self.logger.log_message(msg, color="red") return False diff --git a/src/petab_gui/models/pandas_table_model.py b/src/petab_gui/models/pandas_table_model.py index 305513b..76acfea 100644 --- a/src/petab_gui/models/pandas_table_model.py +++ b/src/petab_gui/models/pandas_table_model.py @@ -174,87 +174,81 @@ def setData(self, index, value, role=Qt.EditRole): def _set_data_single(self, index, value): """Set the data of a single cell.""" - col_setoff = 0 - if self._has_named_index: - col_setoff = 1 - if index.row() == self._data_frame.shape[0]: - # empty row at the end - self.insertRows(index.row(), 1) - self.fill_defaults.emit(index) - next_index = self.index(index.row(), 0) - self.inserted_row.emit(next_index) - if index.column() == 0 and self._has_named_index: - return self.handle_named_index(index, value) row, column = index.row(), index.column() - # Handling non-index (regular data) columns - column_name = self._data_frame.columns[column - col_setoff] - old_value = self._data_frame.iloc[row, column - col_setoff] - # cast to numeric if necessary - expected_type = self._allowed_columns.get(column_name, None) - if is_invalid(value): - change = { - (self._data_frame.index[row], column_name): (old_value, None)} - self.undo_stack.push(ModifyDataFrameCommand(self, change)) + fill_with_defaults = False + + # Handle new row creation + if row == self._data_frame.shape[0]: + self.insertRows(row, 1) + fill_with_defaults = True + next_index = self.index(row, 0) + self.inserted_row.emit(next_index) + + # Handle named index column + if column == 0 and self._has_named_index: + return_this = self.handle_named_index(index, value) + if fill_with_defaults: + self.get_default_values(index) self.cell_needs_validation.emit(row, column) - self.dataChanged.emit(index, index, [Qt.DisplayRole]) + return return_this + + column_name = self._data_frame.columns[column - self.column_offset] + old_value = self._data_frame.iloc[row, column - self.column_offset] + + # Handle invalid value + if is_invalid(value): + self._push_change_and_notify(row, column, column_name, old_value, None) return True - if expected_type: - expected_type = expected_type["type"] - value, error_message = validate_value(value, expected_type) - if error_message: + + # Type validation + expected_info = self._allowed_columns.get(column_name) + if expected_info: + expected_type = expected_info["type"] + validated, error = validate_value(value, expected_type) + if error: self.new_log_message.emit( - f"Column '{column_name}' expects a numeric value", - "red" + f"Column '{column_name}' expects a value of type " + f"{expected_type}, but got '{value}'", "red" ) return False + value = validated + if value == old_value: return False + # Special ID emitters if column_name == "observableId": - change = { - (self._data_frame.index[row], column_name): (old_value, value)} - self.undo_stack.push(ModifyDataFrameCommand(self, change)) - self.dataChanged.emit(index, index, [Qt.DisplayRole]) + self._push_change_and_notify(row, column, column_name, old_value, value) self.relevant_id_changed.emit(value, old_value, "observable") - self.cell_needs_validation.emit(row, column) - self.something_changed.emit(True) + if fill_with_defaults: + self.get_default_values(index) return True - if column_name in ["conditionId", "simulationConditionId", - "preequilibrationConditionId"]: - change = { - (self._data_frame.index[row], column_name): (old_value, value)} - self.undo_stack.push(ModifyDataFrameCommand(self, change)) - self.dataChanged.emit(index, index, [Qt.DisplayRole]) + + if column_name in ["conditionId", "simulationConditionId", "preequilibrationConditionId"]: + if fill_with_defaults: + self.get_default_values(index) + self._push_change_and_notify(row, column, column_name, old_value, value) self.relevant_id_changed.emit(value, old_value, "condition") - self.cell_needs_validation.emit(row, column) - self.something_changed.emit(True) return True - # Validate data based on expected type - expected_type = self._allowed_columns.get(column_name, None) - if expected_type: - expected_type = expected_type["type"] - tried_value = value - value, error_message = validate_value( - value, expected_type - ) - if error_message: - self.new_log_message.emit( - f"Column '{column_name}' expects a value of " - f"type {expected_type}, but got '{tried_value}'", - "red" - ) - return False - # Set the new value + # Default value setting + if fill_with_defaults: + self.get_default_values(index) + self._push_change_and_notify(row, column, column_name, old_value, value) + return True + + def _push_change_and_notify( + self, row, column, column_name, old_value, new_value + ): + """Push a dataframe change to undo stack and notify view + validation.""" change = { - (self._data_frame.index[row], column_name): (old_value, value)} + (self._data_frame.index[row], column_name): (old_value, new_value) + } self.undo_stack.push(ModifyDataFrameCommand(self, change)) - # Validate the row after setting data self.cell_needs_validation.emit(row, column) + self.dataChanged.emit(self.index(row, column), self.index(row, column), + [Qt.DisplayRole]) self.something_changed.emit(True) - self.dataChanged.emit(index, index, [Qt.DisplayRole]) - - return True def handle_named_index(self, index, value): """Handle the named index column.""" @@ -455,8 +449,8 @@ def mimeData(self, rectangle, start_index): def setDataFromText(self, text, start_row, start_column): """Set the data from text.""" - # TODO: Does this need to be more flexible in the separator? lines = text.split("\n") + self.undo_stack.beginMacro("Paste from Clipboard") self.maybe_add_rows(start_row, len(lines)) for row_offset, line in enumerate(lines): values = line.split("\t") @@ -470,6 +464,7 @@ def setDataFromText(self, text, start_row, start_column): value, Qt.EditRole ) + self.undo_stack.endMacro() def maybe_add_rows(self, start_row, n_rows): """Add rows if needed.""" @@ -522,8 +517,6 @@ def fill_row(self, row_position: int, data: dict): data: The data to fill the row with. Gets updated with default values. """ - # TODO: Needs undo command, needs syncing with other commands - # TODO: When undoing a lne and adding then meas -> add both lines data_to_add = { column_name: "" for column_name in self._data_frame.columns } @@ -626,17 +619,23 @@ def handle_named_index(self, index, value): if value == old_value: return False if value in self._data_frame.index: + base = 0 + value = None + while value is None: + idx = f"new_{self.table_type}_{base}" + if idx not in set(self._data_frame.index.astype(str)): + value = idx + base += 1 self.new_log_message.emit( - f"Duplicate index value '{value}'", - "red" + f"Duplicate index value '{value}'. Renaming to default " + f"value '{value}'", + "orange" ) - return False try: self.undo_stack.push(RenameIndexCommand( self, old_value, value, index )) self.relevant_id_changed.emit(value, old_value, self.table_type) - self.cell_needs_validation.emit(row, 0) self.something_changed.emit(True) return True except Exception as e: From b096365e884c1fdd3729e094d10866f6376b9a25 Mon Sep 17 00:00:00 2001 From: PaulJonasJost Date: Thu, 24 Apr 2025 11:27:21 +0200 Subject: [PATCH 9/9] Worked in suggestions --- .../controllers/mother_controller.py | 27 ++++++++++--------- src/petab_gui/views/task_bar.py | 13 ++++----- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/petab_gui/controllers/mother_controller.py b/src/petab_gui/controllers/mother_controller.py index e68ffde..3f28e43 100644 --- a/src/petab_gui/controllers/mother_controller.py +++ b/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, QUndoStack +from PySide6.QtGui import QAction, QDesktopServices, QUndoStack, QKeySequence import zipfile import tempfile import os @@ -194,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") ) @@ -226,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( @@ -379,15 +379,18 @@ def setup_actions(self): qta.icon("mdi6.undo"), "&Undo", self.view ) - actions["undo"].setShortcut("Ctrl+Z") + 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 ) - # set shortcut as ctrl + z + Arrow up - actions["redo"].setShortcut("Ctrl+Shift+Z") + 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): diff --git a/src/petab_gui/views/task_bar.py b/src/petab_gui/views/task_bar.py index 81a8226..f7d5733 100644 --- a/src/petab_gui/views/task_bar.py +++ b/src/petab_gui/views/task_bar.py @@ -65,17 +65,18 @@ def menu_name(self): def __init__(self, parent, actions): super().__init__(parent, actions) - # Find and Replace - self.menu.addAction(actions["find"]) - self.menu.addAction(actions["find+replace"]) + # Undo, Redo + self.menu.addAction(actions["undo"]) + self.menu.addAction(actions["redo"]) self.menu.addSeparator() # Copy, Paste self.menu.addAction(actions["copy"]) self.menu.addAction(actions["paste"]) self.menu.addSeparator() - # Undo, Redo - self.menu.addAction(actions["undo"]) - self.menu.addAction(actions["redo"]) + # Find and Replace + self.menu.addAction(actions["find"]) + self.menu.addAction(actions["find+replace"]) + self.menu.addSeparator() # Add Columns self.menu.addAction(actions["add_column"]) self.menu.addAction(actions["delete_column"])