Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 45 additions & 20 deletions datalab/gui/panel/history/chain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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):
Expand All @@ -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:
Expand Down
9 changes: 7 additions & 2 deletions datalab/gui/panel/history/chainmodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
7 changes: 6 additions & 1 deletion datalab/gui/panel/history/reconnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
8 changes: 2 additions & 6 deletions datalab/locale/fr/LC_MESSAGES/datalab.po
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br><br>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.<br><br>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 ?"
Expand Down
86 changes: 82 additions & 4 deletions datalab/tests/features/common/history_model_unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []
Expand All @@ -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 = []
Expand All @@ -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 = (
Expand Down
52 changes: 52 additions & 0 deletions datalab/tests/features/common/history_workflow_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 7 additions & 3 deletions doc/features/common/historypanel.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 20 additions & 8 deletions doc/locale/fr/LC_MESSAGES/features/common/historypanel.po
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
6 changes: 6 additions & 0 deletions doc/locale/fr/LC_MESSAGES/release_notes/release_1.04.po
Original file line number Diff line number Diff line change
Expand Up @@ -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 :**"

Expand Down
Loading
Loading