From 4b77373778e0bd57606835c916e170ed7dd1aa07 Mon Sep 17 00:00:00 2001 From: Alessio Buccino Date: Tue, 29 Sep 2026 16:35:56 +0200 Subject: [PATCH] fix: serialize panel curation changes on the event loop --- spikeinterface_gui/backend_panel.py | 15 +++- spikeinterface_gui/curationview.py | 31 +++++-- spikeinterface_gui/unitlistview.py | 122 ++++++++++++++++++---------- spikeinterface_gui/utils_panel.py | 28 +++++++ 4 files changed, 139 insertions(+), 57 deletions(-) diff --git a/spikeinterface_gui/backend_panel.py b/spikeinterface_gui/backend_panel.py index bf28cb8..913fe1f 100644 --- a/spikeinterface_gui/backend_panel.py +++ b/spikeinterface_gui/backend_panel.py @@ -433,11 +433,18 @@ def set_external_curation(self, curation_data): if "curation" not in self.views: return + from .utils_panel import schedule_curation_change + curation_view = self.views["curation"] - self.controller.set_curation_data(curation_data) - curation_view.notify_manual_curation_updated() - self.controller.current_curation_saved = True - curation_view.refresh() + + # scheduled like the other curation changes, so it is not interleaved with them + def _apply(): + self.controller.set_curation_data(curation_data) + curation_view.notify_manual_curation_updated() + self.controller.current_curation_saved = True + curation_view.refresh() + + schedule_curation_change(_apply) # we also need to refresh the unit list view to update the unit visibility according to the new curation if "unitlist" in self.views: diff --git a/spikeinterface_gui/curationview.py b/spikeinterface_gui/curationview.py index 2096380..b9e4a5b 100644 --- a/spikeinterface_gui/curationview.py +++ b/spikeinterface_gui/curationview.py @@ -547,10 +547,14 @@ def _panel_update_unit_visibility(self, event): def _panel_restore_units(self, event): - self.restore_units() + from .utils_panel import schedule_curation_change + + schedule_curation_change(self.restore_units) def _panel_unmerge(self, event): - self.unmerge() + from .utils_panel import schedule_curation_change + + schedule_curation_change(self.unmerge) def _panel_apply_curation_to_analyzer(self, event): self.apply_curation_to_analyzer() @@ -559,11 +563,20 @@ def _panel_restore_original_analyzer(self, event): self.restore_original_analyzer() def _panel_unsplit(self, event): - self.unsplit() + from .utils_panel import schedule_curation_change + + schedule_curation_change(self.unsplit) def _panel_save_curation_callback(self, event): - self.controller.save_curation_callback() - self.refresh() + from .utils_panel import schedule_curation_change + + # scheduled like the other curation changes, so that all the changes + # received before the click are applied when the curation is saved + def _apply(): + self.controller.save_curation_callback() + self.refresh() + + schedule_curation_change(_apply) def _panel_generate_json(self): # Get the path from the text input @@ -601,12 +614,14 @@ def _panel_get_split_table_row(self): return selected_items def _panel_handle_shortcut(self, event): + from .utils_panel import schedule_curation_change + if event.data == "restore": - self.restore_units() + schedule_curation_change(self.restore_units) elif event.data == "unmerge": - self.unmerge() + schedule_curation_change(self.unmerge) elif event.data == "unsplit": - self.unsplit() + schedule_curation_change(self.unsplit) def _panel_on_unit_visibility_changed(self): for table in [self.table_delete, self.table_merge, self.table_split]: diff --git a/spikeinterface_gui/unitlistview.py b/spikeinterface_gui/unitlistview.py index 89124ec..329104f 100644 --- a/spikeinterface_gui/unitlistview.py +++ b/spikeinterface_gui/unitlistview.py @@ -793,17 +793,26 @@ def _panel_on_unit_color_changed(self): self.refresh() def _panel_on_edit(self, event): + from .utils_panel import schedule_curation_change + column = event.column if self.label_definitions is not None and column in self.label_definitions: row = event.row - unit_id = self.table.value.index[row] - new_label = event.value - if new_label == "": - new_label = None - self.controller.set_label_to_unit(unit_id, column, new_label) - self.notify_manual_curation_updated() - # like in Qt, move to the next unit and make it visible alone - self.table.select_next_row(only=True, from_row=row) + + def _apply(): + unit_id = self.table.value.index[row] + # event.value is filled from the table data on a worker thread and can be stale + # when events are processed concurrently: read it again here + new_label = self.table.value[column].iloc[row] + if new_label == "": + new_label = None + self.controller.set_label_to_unit(unit_id, column, new_label) + self.notify_manual_curation_updated() + # like in Qt, move to the next unit and make it visible alone + self.table.select_next_row(only=False, from_row=row) + return self._panel_on_only_selection + + schedule_curation_change(_apply) self.notifier.notify_active_view_updated() def _panel_update_labels(self): @@ -839,49 +848,72 @@ def _panel_get_selected_unit_ids(self): return unit_ids[self.table.selection] def _panel_delete_unit(self): - removed_unit_ids = self.get_selected_unit_ids() - success = self.controller.make_manual_delete_if_possible(removed_unit_ids) - if not success: - self.warning("Delete could not be performed. Ensure unit ids are not removed or merged already.") - return - self.notify_manual_curation_updated() - self.refresh() + from .utils_panel import schedule_curation_change + + def _apply(): + removed_unit_ids = self.get_selected_unit_ids() + success = self.controller.make_manual_delete_if_possible(removed_unit_ids) + if not success: + self.warning("Delete could not be performed. Ensure unit ids are not removed or merged already.") + return + self.notify_manual_curation_updated() + self.refresh() + + schedule_curation_change(_apply) def _panel_merge_units(self): - merge_unit_ids = self.get_selected_unit_ids() - success = self.controller.make_manual_merge_if_possible(merge_unit_ids) - if not success: - self.warning( - "Merge could not be performed. Ensure unit ids are not removed " - "merged, or split already." - ) - return - self.notify_manual_curation_updated() - self.refresh() + from .utils_panel import schedule_curation_change + + def _apply(): + merge_unit_ids = self.get_selected_unit_ids() + success = self.controller.make_manual_merge_if_possible(merge_unit_ids) + if not success: + self.warning( + "Merge could not be performed. Ensure unit ids are not removed " + "merged, or split already." + ) + return + self.notify_manual_curation_updated() + self.refresh() + + schedule_curation_change(_apply) def _panel_remove_from_merge(self): - merge_unit_ids = self.get_selected_unit_ids() - success = self.controller.remove_units_from_merge_if_possible(merge_unit_ids) - if not success: - self.warning( - "Could not remove units from a merge. Ensure all selected units are in a merge " - "group, and that you are not leaving zero or one units in the merge group." - ) - return - self.notify_manual_curation_updated() - self.refresh() + from .utils_panel import schedule_curation_change + + def _apply(): + merge_unit_ids = self.get_selected_unit_ids() + success = self.controller.remove_units_from_merge_if_possible(merge_unit_ids) + if not success: + self.warning( + "Could not remove units from a merge. Ensure all selected units are in a merge " + "group, and that you are not leaving zero or one units in the merge group." + ) + return + self.notify_manual_curation_updated() + self.refresh() + + schedule_curation_change(_apply) def _panel_set_default_label(self, label): - selected_unit_ids = self._panel_get_selected_unit_ids() - if len(selected_unit_ids) == 0: - return - for unit_id in selected_unit_ids: - self.controller.set_label_to_unit(unit_id, "quality", label) - self.table.value.loc[selected_unit_ids, "quality"] = label if label is not None else "" - self.notify_manual_curation_updated() - self.refresh() - # like in Qt, move to the next unit and make it visible alone - self.table.select_next_row(only=True) + from .utils_panel import schedule_curation_change + + def _apply(): + # the selection is read here (on the event loop), so it reflects the + # select_next_row of the previous label change + selected_unit_ids = self._panel_get_selected_unit_ids() + if len(selected_unit_ids) == 0: + return + for unit_id in selected_unit_ids: + self.controller.set_label_to_unit(unit_id, "quality", label) + self.table.value.loc[selected_unit_ids, "quality"] = label if label is not None else "" + self.notify_manual_curation_updated() + self.refresh() + # like in Qt, move to the next unit and make it visible alone + self.table.select_next_row(only=False) + return self._panel_on_only_selection + + schedule_curation_change(_apply) def _panel_handle_shortcut(self, event): if self.is_view_active(): diff --git a/spikeinterface_gui/utils_panel.py b/spikeinterface_gui/utils_panel.py index 6122897..61445c3 100644 --- a/spikeinterface_gui/utils_panel.py +++ b/spikeinterface_gui/utils_panel.py @@ -22,6 +22,34 @@ from .view_base import ViewBase +def schedule_curation_change(apply_change): + """ + Run a curation change serialized on the server event loop. + + When Panel runs with ``pn.config.nthreads``, client events (table edits, + shortcuts, button clicks) are dispatched concurrently on a thread pool, so + handlers that read the table state and mutate ``controller.curation_data`` + can race with each other and with "Save curation". Scheduling the change on + the event loop runs these changes one at a time, in the order they were + received. + + Parameters + ---------- + apply_change : callable + Cheap function that reads the current state and applies the curation + change. It can return a callable with follow-up (heavy) work, which is + run on the thread pool (if available) so it does not block the event + loop shared by all sessions. + """ + + def _apply(): + follow_up = apply_change() + if follow_up is not None: + pn.state.execute(follow_up, schedule="thread" if pn.state._thread_pool else True) + + pn.state.execute(_apply, schedule=True) + + _bg_color = "#181818"