diff --git a/datalab/gui/panel/history/chain.py b/datalab/gui/panel/history/chain.py index 3a2691adc..15c769b59 100644 --- a/datalab/gui/panel/history/chain.py +++ b/datalab/gui/panel/history/chain.py @@ -377,13 +377,19 @@ def remove_single_action(panel: HistoryPanel, action: HistoryAction) -> None: def find_reconnection_source( panel: HistoryPanel, panel_str: str, output_uuid: str ) -> tuple[HistoryAction | None, str | None]: - """Return the action and source UUID behind a removed output.""" + """Return the action and source UUID behind a removed output. + + Only a processing step has a source: an object created or loaded by a UI + action has none, whatever the selection recorded with the action. + """ action_uuid = panel.runtime.objects.output_to_action.get(output_uuid) if action_uuid is None: return None, None for session in panel.history_sessions: for action in session.actions: if action.uuid == action_uuid: + if action.kind != HistoryAction.KIND_COMPUTE: + return action, None selection = action.state.selection.get(panel_str, []) source_uuid = selection[0] if selection else None return action, source_uuid @@ -423,16 +429,15 @@ def plan_reconnection( return plan alive_ids = set(panel_data.objmodel.get_object_ids()) if source_uuid is None or source_uuid not in alive_ids: - label = removed_uuid - if producer_action is not None: - label = producer_action.title or producer_action.func_name or removed_uuid - plan.warning = ( - _( - "“%s” has dependent operations but no valid source to " - "reconnect to — downstream results are left unchanged." + # Objects that were not produced by a recorded processing step (raw + # data, or history not recording) have no source: nothing to report. + if ( + producer_action is not None + and producer_action.kind == HistoryAction.KIND_COMPUTE + ): + plan.unreconnected_operation = ( + producer_action.title or producer_action.func_name ) - % label - ) return plan if producer_action is not None: outputs = panel.runtime.objects.action_output_uuids.get( @@ -449,7 +454,7 @@ def apply_reconnection_plan( roots_to_recompute: list[HistoryAction], ) -> None: """Apply object and action source rewrites described by ``plan``.""" - if plan.warning is not None or plan.source_uuid is None: + if plan.source_uuid is None or not panel_data.objmodel.has_uuid(plan.source_uuid): return for target in plan.targets: if not panel_data.objmodel.has_uuid(target.object_uuid): @@ -474,16 +479,36 @@ def apply_reconnection_plan( remove_single_action(panel, plan.producer_action) -def show_reconnection_warnings(panel: HistoryPanel, warnings: list[str]) -> None: - """Show reconnection warnings at the GUI boundary.""" - if warnings and not execenv.unattended: - QW.QMessageBox.warning( - panel.mainwindow, - _("Delete"), - _("Some operations could not be reconnected after deletion:") - + "\n\n• " - + "\n• ".join(warnings), +#: Maximum number of operations listed by the reconnection warning +MAX_LISTED_UNRECONNECTED = 10 + + +def show_reconnection_warnings(panel: HistoryPanel, operations: list[str]) -> None: + """Show, in a single message, the operations that could not be reconnected. + + Args: + panel: History panel + operations: titles of the processing steps whose deleted outputs had + dependent results but no remaining source (listed once each, at most + :data:`MAX_LISTED_UNRECONNECTED` of them) + """ + unique = list(dict.fromkeys(operations)) + if not unique or execenv.unattended: + return + lines = ["• " + title for title in unique[:MAX_LISTED_UNRECONNECTED]] + if len(unique) > MAX_LISTED_UNRECONNECTED: + lines.append(_("... and %d more") % (len(unique) - MAX_LISTED_UNRECONNECTED)) + QW.QMessageBox.warning( + panel.mainwindow, + _("Delete"), + _( + "The outputs of the following operations were deleted, but their " + "source no longer exists: the results depending on them could not " + "be reconnected and are left unchanged." ) + + "\n\n" + + "\n".join(lines), + ) def refresh_reconnected_history(panel: HistoryPanel) -> None: diff --git a/datalab/gui/panel/history/chainmodel.py b/datalab/gui/panel/history/chainmodel.py index 7173b2837..3903bb4ba 100644 --- a/datalab/gui/panel/history/chainmodel.py +++ b/datalab/gui/panel/history/chainmodel.py @@ -96,14 +96,19 @@ class ReconnectionTarget: @dataclass class ReconnectionPlan: - """Planned source rewrites after one data object has been removed.""" + """Planned source rewrites after one data object has been removed. + + ``unreconnected_operation`` is the title of the recorded processing step that + produced the removed object when its dependent results cannot be reconnected + because its source no longer exists (None otherwise). + """ panel_str: str removed_uuid: str source_uuid: str | None producer_action: HistoryAction | None targets: list[ReconnectionTarget] = field(default_factory=list) - warning: str | None = None + unreconnected_operation: str | None = None remove_producer: bool = False diff --git a/datalab/gui/panel/history/reconnection.py b/datalab/gui/panel/history/reconnection.py index c2c88d0fa..bbb7270bf 100644 --- a/datalab/gui/panel/history/reconnection.py +++ b/datalab/gui/panel/history/reconnection.py @@ -42,6 +42,11 @@ def reconnect_chain_after_removal( if success: hrec.recompute_cascade(panel, action) hchain.show_reconnection_warnings( - panel, [plan.warning for plan in plans if plan.warning is not None] + panel, + [ + plan.unreconnected_operation + for plan in plans + if plan.unreconnected_operation is not None + ], ) hchain.refresh_reconnected_history(panel) diff --git a/datalab/locale/fr/LC_MESSAGES/datalab.po b/datalab/locale/fr/LC_MESSAGES/datalab.po index 067eb4dac..0cff789c3 100644 --- a/datalab/locale/fr/LC_MESSAGES/datalab.po +++ b/datalab/locale/fr/LC_MESSAGES/datalab.po @@ -1904,12 +1904,8 @@ msgstr "Annotation ajoutée" msgid "The label has been added as an annotation. You can edit or remove it using the annotation editing window.

Choosing to ignore this message will prevent it from being displayed again." msgstr "L'étiquette a été ajoutée comme annotation. Vous pouvez la modifier ou la supprimer en utilisant la fenêtre d'édition des annotations.

Ignorer ce message empêchera son affichage ultérieur." -#, python-format -msgid "“%s” has dependent operations but no valid source to reconnect to — downstream results are left unchanged." -msgstr "« %s » possède des opérations dépendantes mais aucune source valide à laquelle se reconnecter — les résultats en aval restent inchangés." - -msgid "Some operations could not be reconnected after deletion:" -msgstr "Certaines opérations n'ont pas pu être reconnectées après la suppression :" +msgid "The outputs of the following operations were deleted, but their source no longer exists: the results depending on them could not be reconnected and are left unchanged." +msgstr "Les sorties des opérations suivantes ont été supprimées, mais leur source n'existe plus : les résultats qui en dépendent n'ont pas pu être reconnectés et restent inchangés." msgid "Commit edit mode changes?" msgstr "Valider les modifications du mode d'édition ?" diff --git a/datalab/tests/features/common/history_model_unit_test.py b/datalab/tests/features/common/history_model_unit_test.py index 634e105d0..b5ce9efcb 100644 --- a/datalab/tests/features/common/history_model_unit_test.py +++ b/datalab/tests/features/common/history_model_unit_test.py @@ -1290,10 +1290,10 @@ def make_panel(objects: list) -> tuple[SimpleNamespace, SimpleNamespace]: ) return panel, signal_panel - # Dead source: the plan carries a warning and applying it is a no-op + # Dead source: the plan reports the producer and applying it is a no-op panel, signal_panel = make_panel([consumer_obj]) plan = hchain.plan_reconnection(panel, signal_panel, removed_uuid) - assert plan.warning is not None and "Normalize" in plan.warning + assert plan.unreconnected_operation == "Normalize" assert [target.object_uuid for target in plan.targets] == [consumer_uuid] assert plan.targets[0].action is consumer_action roots: list[HistoryAction] = [] @@ -1305,13 +1305,13 @@ def make_panel(objects: list) -> tuple[SimpleNamespace, SimpleNamespace]: patch.object(hchain, "execenv", SimpleNamespace(unattended=True)), patch.object(hchain.QW, "QMessageBox") as message_box, ): - hchain.show_reconnection_warnings(panel, [plan.warning]) + hchain.show_reconnection_warnings(panel, [plan.unreconnected_operation]) message_box.warning.assert_not_called() # Alive source: reconnect the consumer and remove the outputless producer panel, signal_panel = make_panel([source, consumer_obj]) plan = hchain.plan_reconnection(panel, signal_panel, removed_uuid) - assert plan.warning is None + assert plan.unreconnected_operation is None assert plan.source_uuid == source_uuid assert plan.remove_producer is True roots = [] @@ -1323,6 +1323,84 @@ def make_panel(objects: list) -> tuple[SimpleNamespace, SimpleNamespace]: panel.runtime.objects.remove_action_outputs.assert_called_once_with(producer) +def test_plan_reconnection_ignores_objects_without_processing_source() -> None: + """Deleting a raw object reports nothing and reconnects nothing, even when + its creation action recorded a selection or history was not recording.""" + unrelated = create_paracetamol_signal() + removed_uuid = "removed-raw" + consumer_obj = create_paracetamol_signal() + insert_processing_parameters( + consumer_obj, + ProcessingParameters( + func_name="normalize", pattern="1-to-1", source_uuid=removed_uuid + ), + ) + creation = HistoryAction( + title="New signal", + kind=HistoryAction.KIND_UI, + panel_str="signal", + target="signalpanel", + method_name="new_object", + state=build_workspace_state([get_uuid(unrelated)]), + ) + session = HistorySession(number=1) + session.add_action(creation) + signal_panel = SimpleNamespace( + PANEL_STR_ID="signal", objmodel=CascadeObjectModel([unrelated, consumer_obj]) + ) + # Not recorded (no producer), then recorded by a creation action + for output_to_action in ({}, {removed_uuid: creation.uuid}): + panel = SimpleNamespace( + history_sessions=[session], + runtime=SimpleNamespace( + objects=SimpleNamespace( + output_to_action=output_to_action, action_output_uuids={} + ) + ), + mainwindow=SimpleNamespace( + signalpanel=signal_panel, + imagepanel=SimpleNamespace( + PANEL_STR_ID="image", objmodel=CascadeObjectModel([]) + ), + ), + ) + plan = hchain.plan_reconnection(panel, signal_panel, removed_uuid) + assert len(plan.targets) == 1 + assert plan.source_uuid is None + assert plan.unreconnected_operation is None + roots: list[HistoryAction] = [] + hchain.apply_reconnection_plan(panel, signal_panel, plan, roots) + assert not roots + pp = extract_processing_parameters(consumer_obj) + assert pp.source_uuid == removed_uuid + assert creation in session.actions + + +def test_show_reconnection_warnings_lists_each_operation_once() -> None: + """One message lists each operation once, up to a maximum number.""" + panel = SimpleNamespace(mainwindow=None) + with ( + patch.object(hchain, "execenv", SimpleNamespace(unattended=False)), + patch.object(hchain.QW, "QMessageBox") as message_box, + ): + hchain.show_reconnection_warnings(panel, []) + message_box.warning.assert_not_called() + + hchain.show_reconnection_warnings(panel, ["Normalize"] * 5 + ["Derivative"]) + message_box.warning.assert_called_once() + text = message_box.warning.call_args.args[2] + assert text.count("• ") == 2 + assert text.count("Normalize") == text.count("Derivative") == 1 + + message_box.warning.reset_mock() + count = hchain.MAX_LISTED_UNRECONNECTED + 3 + hchain.show_reconnection_warnings(panel, [f"Op {i}" for i in range(count)]) + text = message_box.warning.call_args.args[2] + assert text.count("• ") == hchain.MAX_LISTED_UNRECONNECTED + assert f"Op {hchain.MAX_LISTED_UNRECONNECTED}" not in text + assert "3" in text.splitlines()[-1] + + def test_prepare_action_param_edit_skips_paramless_actions() -> None: """Return no edit target (and skip the dialog) for param-less actions.""" paramless = ( diff --git a/datalab/tests/features/common/history_workflow_test.py b/datalab/tests/features/common/history_workflow_test.py index 956360711..b52768666 100644 --- a/datalab/tests/features/common/history_workflow_test.py +++ b/datalab/tests/features/common/history_workflow_test.py @@ -1079,6 +1079,58 @@ def test_replay_recreates_deleted_output_under_recorded_uuid() -> None: assert parameters.source_uuid == source_uuid +@pytest.mark.parametrize("record", (False, True)) +def test_group_deletion_warns_only_for_unreconnectable_operations( + record: bool, +) -> None: + """Deleting raw signals whose results live in another group shows no message; + deleting the processed group then lists its recorded operation only once.""" + with datalab_test_app_context(history=True) as win: + history, panel = win.historypanel, win.signalpanel + history.toggle_record_mode(record) + raw_group = panel.add_group("Raw") + for _index in range(3): + win.add_object(create_paracetamol_signal(), get_uuid(raw_group)) + raw_uuids = panel.objmodel.get_object_ids() + panel.objview.select_groups([get_uuid(raw_group)]) + panel.processor.run_feature( + sips.normalize, sigima.params.NormalizeParam.create(method="maximum") + ) + normalize_title = history[len(history)].title if record else None + normalized_group = panel.objmodel.get_groups()[-1] + normalized_uuids = [get_uuid(obj) for obj in normalized_group.get_objects()] + panel.objview.select_groups([get_uuid(normalized_group)]) + panel.processor.run_feature( + sips.moving_average, sigima.params.MovingAverageParam.create(n=3) + ) + averaged_group = panel.objmodel.get_groups()[-1] + averaged_uuids = [get_uuid(obj) for obj in averaged_group.get_objects()] + with ( + patch.object(hchain, "execenv", SimpleNamespace(unattended=False)), + patch.object(hchain.QW, "QMessageBox") as message_box, + ): + panel.objview.select_groups([get_uuid(raw_group)]) + panel.remove_object(force=True) + message_box.warning.assert_not_called() + assert [ + extract_processing_parameters(panel.objmodel[uuid]).source_uuid + for uuid in normalized_uuids + ] == raw_uuids + + panel.objview.select_groups([get_uuid(normalized_group)]) + panel.remove_object(force=True) + if record: + message_box.warning.assert_called_once() + text = message_box.warning.call_args.args[2] + assert text.count("• ") == 1 and text.count(normalize_title) == 1 + else: + message_box.warning.assert_not_called() + assert [ + extract_processing_parameters(panel.objmodel[uuid]).source_uuid + for uuid in averaged_uuids + ] == normalized_uuids + + def test_deletion_reconnects_and_splices_chain() -> None: """Reconnect after data deletion, then splice the producing action.""" with datalab_test_app_context(history=True) as win: diff --git a/doc/features/common/historypanel.rst b/doc/features/common/historypanel.rst index e5c564dcc..d036ada1a 100644 --- a/doc/features/common/historypanel.rst +++ b/doc/features/common/historypanel.rst @@ -363,9 +363,13 @@ chain: the source of the deleted step as their new input. - For ``2_to_1`` operations (e.g. *difference*), the first source is used for reconnection. -- If no valid source can be determined (e.g. the source itself was already - deleted), a warning is displayed listing the unreconnectable operations, - but the deletion is allowed to proceed. +- If the source of the deleted step no longer exists (e.g. it was already + deleted, or is deleted at the same time), the deletion proceeds and the + dependent results are left unchanged: a single warning lists the processing + steps concerned, each of them once. +- Deleting an object that was not produced by a recorded processing step + (e.g. a created or loaded object, or any object when recording was off) + leaves the results depending on it unchanged, without any warning. This behaviour mirrors removing a link from a chain: the adjacent links reconnect to preserve the processing flow. diff --git a/doc/locale/fr/LC_MESSAGES/features/common/historypanel.po b/doc/locale/fr/LC_MESSAGES/features/common/historypanel.po index f04d6384c..8dd168345 100644 --- a/doc/locale/fr/LC_MESSAGES/features/common/historypanel.po +++ b/doc/locale/fr/LC_MESSAGES/features/common/historypanel.po @@ -741,14 +741,26 @@ msgstr "" "source est utilisée pour la reconnexion." msgid "" -"If no valid source can be determined (e.g. the source itself was already " -"deleted), a warning is displayed listing the unreconnectable operations, " -"but the deletion is allowed to proceed." -msgstr "" -"Si aucune source valide ne peut être déterminée (par exemple la source " -"elle-même a déjà été supprimée), un avertissement est affiché listant les" -" opérations non reconnectables, mais la suppression est néanmoins " -"autorisée." +"If the source of the deleted step no longer exists (e.g. it was already " +"deleted, or is deleted at the same time), the deletion proceeds and the " +"dependent results are left unchanged: a single warning lists the " +"processing steps concerned, each of them once." +msgstr "" +"Si la source de l'étape supprimée n'existe plus (par exemple si elle a " +"déjà été supprimée, ou si elle est supprimée en même temps), la " +"suppression a lieu et les résultats dépendants restent inchangés : un " +"unique avertissement liste les étapes de traitement concernées, chacune " +"une seule fois." + +msgid "" +"Deleting an object that was not produced by a recorded processing step " +"(e.g. a created or loaded object, or any object when recording was off) " +"leaves the results depending on it unchanged, without any warning." +msgstr "" +"La suppression d'un objet qui n'a pas été produit par une étape de " +"traitement enregistrée (par exemple un objet créé ou chargé, ou tout " +"objet lorsque l'enregistrement était désactivé) laisse inchangés les " +"résultats qui en dépendent, sans aucun avertissement." msgid "" "This behaviour mirrors removing a link from a chain: the adjacent links " diff --git a/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po b/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po index 4214b2d27..66089c6f7 100644 --- a/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po +++ b/doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po @@ -548,6 +548,12 @@ msgstr "**Plugins et historique :**" msgid "Replaying a History action or re-processing a result created by a plugin feature that shares its name with a built-in feature now runs the plugin feature instead of the built-in one" msgstr "Le rejeu d'une action de l'historique ou le recalcul d'un résultat créé par une fonctionnalité de plugin portant le même nom qu'une fonctionnalité intégrée exécute désormais la fonctionnalité du plugin au lieu de la fonctionnalité intégrée" +msgid "**History Panel:**" +msgstr "**Panneau d'historique :**" + +msgid "Fixed an oversized warning appearing when deleting a group of signals or images whose processing results were kept in another group, even when the History panel was not recording: it listed every deleted object, often as an unreadable identifier, and stated that \"operations could not be reconnected\" although there was nothing to reconnect. Deleting such objects now shows no message, and the results depending on them are left unchanged as before. The warning is now shown only when a recorded processing step loses its source, and it lists each step once, in a message of bounded size (fixes [Issue #451](https://github.com/DataLab-Platform/DataLab/issues/451))" +msgstr "Correction d'un avertissement démesuré qui s'affichait lors de la suppression d'un groupe de signaux ou d'images dont les résultats de traitement étaient conservés dans un autre groupe, même lorsque le panneau d'historique n'enregistrait pas : il listait chaque objet supprimé, souvent sous la forme d'un identifiant illisible, et indiquait que des « opérations n'avaient pas pu être reconnectées » alors qu'il n'y avait rien à reconnecter. La suppression de tels objets n'affiche plus aucun message, et les résultats qui en dépendent restent inchangés, comme auparavant. L'avertissement n'est désormais affiché que lorsqu'une étape de traitement enregistrée perd sa source, et il liste chaque étape une seule fois, dans un message de taille limitée (corrige [l'issue #451](https://github.com/DataLab-Platform/DataLab/issues/451))" + msgid "**Object titles:**" msgstr "**Titres des objets :**" diff --git a/doc/release_notes/release_1.04.md b/doc/release_notes/release_1.04.md index 219d72e2a..613de9532 100644 --- a/doc/release_notes/release_1.04.md +++ b/doc/release_notes/release_1.04.md @@ -243,6 +243,10 @@ See the [HDF5 browser guide](../features/common/h5browser.rst) for scientific ob * Replaying a History action or re-processing a result created by a plugin feature that shares its name with a built-in feature now runs the plugin feature instead of the built-in one +**History Panel:** + +* Fixed an oversized warning appearing when deleting a group of signals or images whose processing results were kept in another group, even when the History panel was not recording: it listed every deleted object, often as an unreadable identifier, and stated that "operations could not be reconnected" although there was nothing to reconnect. Deleting such objects now shows no message, and the results depending on them are left unchanged as before. The warning is now shown only when a recorded processing step loses its source, and it lists each step once, in a message of bounded size (fixes [Issue #451](https://github.com/DataLab-Platform/DataLab/issues/451)) + **Object titles:** * Fixed plot legends still showing the original generated title after renaming an object, e.g. a profile extracted from an image (fixes [Issue #323](https://github.com/DataLab-Platform/DataLab/issues/323))