Merge pull request #422 from DataLab-Platform/feature/experimental-data-integration

Integrate experimental data: NeXus files, robust HDF5 import and threaded plugin instruments

Author: PierreRaybaut

datalab/control/baseproxy.py

@@ -401,13 +401,16 @@ def set_object(self, obj: SignalObj | ImageObj) -> None:
     @abc.abstractmethod
     def add_group(
         self, title: str, panel: str | None = None, select: bool = False
-    ) -> None:
+    ) -> str:
         """Add group to DataLab.
 
         Args:
             title: Group title
             panel: Panel name (valid values: "signal", "image"). Defaults to None.
             select: Select the group after creation. Defaults to False.
+
+        Returns:
+            UUID of the created group
         """
 
     @abc.abstractmethod
@@ -923,15 +926,18 @@ def get_current_object_uuid(self) -> str | None:
 
     def add_group(
         self, title: str, panel: str | None = None, select: bool = False
-    ) -> None:
+    ) -> str:
         """Add group to DataLab.
 
         Args:
             title: Group title
             panel: Panel name (valid values: "signal", "image"). Defaults to None.
             select: Select the group after creation. Defaults to False.
+
+        Returns:
+            UUID of the created group
         """
-        self._datalab.add_group(title, panel, select)
+        return self._datalab.add_group(title, panel, select)
 
     def select_objects(
         self,

datalab/control/remote.py

@@ -126,7 +126,6 @@ def __init__(self, win: DLMainWindow) -> None:
         for signal, command in (
             (self.SIG_ADD_OBJECT, win.add_object),
             (self.SIG_SET_OBJECT, win.set_object),
-            (self.SIG_ADD_GROUP, win.add_group),
             (self.SIG_LOAD_FROM_FILES, win.load_from_files),
             (self.SIG_LOAD_FROM_DIRECTORY, win.load_from_directory),
             (self.SIG_SELECT_OBJECTS, win.select_objects),
@@ -149,6 +148,9 @@ def __init__(self, win: DLMainWindow) -> None:
             (self.SIG_IMPORT_MACRO_FROM_FILE, win.import_macro_from_file),
         ):
             signal.connect(functools.partial(win.run_remote_command, command))
+        self.SIG_ADD_GROUP.connect(
+            functools.partial(win.run_remote_command, win.add_group, return_result=True)
+        )
         self.SIG_CALL_METHOD.connect(win.call_method_slot)
 
     def serve(self) -> None:
@@ -572,15 +574,20 @@ def get_current_object_uuid(self) -> str | None:
     @remote_call
     def add_group(
         self, title: str, panel: str | None = None, select: bool = False
-    ) -> None:
+    ) -> str:
         """Add group to DataLab.
 
         Args:
             title: Group title
             panel: Panel name (valid values: "signal", "image"). Defaults to None.
             select: Select the group after creation. Defaults to False.
+
+        Returns:
+            UUID of the created group
         """
         self.SIG_ADD_GROUP.emit(title, panel, select)
+        # The decorator waits for the main window slot, which sets the result
+        return self.result
 
     @remote_call
     def select_objects(

datalab/gui/h5io.py

@@ -68,7 +68,6 @@ class H5InputOutput:
 
     def __init__(self, mainwindow: DLMainWindow) -> None:
         self.mainwindow = mainwindow
-        self.uint32_wng: bool = None
         #: True when the user was already told, for the current opening, that
         #: the provenance of appended workspaces is not imported (no repeat).
         self.append_informed = False
@@ -273,29 +272,36 @@ def __add_object_from_node(self, node: BaseNode) -> None:
         obj = node.get_native_object()
         if obj is None:
             return
-        self.uint32_wng = self.uint32_wng or node.uint32_wng
         if isinstance(obj, SignalObj):
             self.mainwindow.signalpanel.add_object(obj)
         else:
             self.mainwindow.imagepanel.add_object(obj)
 
-    def __eventually_show_warnings(self) -> None:
-        """Eventually show warnings after everything is imported"""
-        if self.uint32_wng:
-            QW.QMessageBox.warning(
-                self.mainwindow, _("Warning"), _("Clipping uint32 data to int32.")
-            )
-
     def import_files(
         self, filenames: list[str], import_all: bool, reset_all: bool
     ) -> None:
         """Import HDF5 files"""
         h5browser = H5BrowserDialog(self.mainwindow)
+        try:
+            self.__import_files_from_browser(
+                h5browser, filenames, import_all, reset_all
+            )
+        finally:
+            # Close the HDF5 files, even if an import error is raised
+            h5browser.cleanup()
+
+    def __import_files_from_browser(
+        self,
+        h5browser: H5BrowserDialog,
+        filenames: list[str],
+        import_all: bool,
+        reset_all: bool,
+    ) -> None:
+        """Open HDF5 files in browser, then import the selected datasets"""
         for filename in filenames:
             with qt_try_loadsave_file(self.mainwindow, filename, "load"):
                 h5browser.open_file(filename)
         if h5browser.is_empty():
-            h5browser.cleanup()
             if not execenv.unattended:
                 QW.QMessageBox.warning(
                     self.mainwindow,
@@ -314,27 +320,52 @@ def import_files(
             if nodes is not None:
                 if reset_all:
                     self.mainwindow.reset_all()
+                errors: list[str] = []
                 with qt_try_loadsave_file(self.mainwindow, "*.h5", "load"):
                     with create_progress_bar(self.mainwindow, "", len(nodes)) as prog:
-                        self.uint32_wng = False
                         for idx, node in enumerate(nodes):
                             prog.setLabelText(self.__progbartitle(node.h5file.filename))
                             prog.setValue(idx + 1)
                             QW.QApplication.processEvents()
                             if prog.wasCanceled():
                                 break
-                            self.__add_object_from_node(node)
-                self.__eventually_show_warnings()
-        h5browser.cleanup()
+                            try:
+                                self.__add_object_from_node(node)
+                            except Exception as exc:  # pylint: disable=broad-except
+                                # Data of third-party files: import the other
+                                # datasets, then report all errors at once
+                                errors.append(f"{node.id}: {exc}")
+                    self.__report_import_errors(errors)
+
+    def __report_import_errors(self, errors: list[str]) -> None:
+        """Report the datasets that could not be imported
+
+        Args:
+            errors: description of each error ("dataset: reason")
+
+        Raises:
+            ValueError: in unattended mode, if any dataset could not be imported
+        """
+        if not errors:
+            return
+        if execenv.unattended:
+            raise ValueError("\n".join(errors))
+        shown = errors[:10]
+        if len(errors) > len(shown):
+            shown.append(_("... and %d more") % (len(errors) - len(shown)))
+        QW.QMessageBox.warning(
+            self.mainwindow,
+            _("Warning"),
+            _("Some datasets could not be imported:") + "\n\n" + "\n".join(shown),
+        )
 
     def import_dataset_from_file(self, filename: str, dsetname: str) -> None:
         """Import dataset from HDF5 file"""
         h5importer = H5Importer(filename)
         try:
             node = h5importer.get(dsetname)
-            self.uint32_wng = False
             self.__add_object_from_node(node)
-            self.__eventually_show_warnings()
         except KeyError as exc:
             raise KeyError(f"Dataset not found: {dsetname}") from exc
-        h5importer.close()
+        finally:
+            h5importer.close()

datalab/gui/main.py

@@ -25,6 +25,7 @@
 import sys
 import time
 import traceback
+from concurrent.futures import Future
 from contextlib import ExitStack
 from datetime import datetime
 from typing import TYPE_CHECKING
@@ -75,7 +76,11 @@
 from datalab.gui.panel import base, history, image, macro, signal
 from datalab.gui.plugins.applications import ApplicationsDialog, get_plugin_icon
 from datalab.gui.plugins.config import PluginConfigDialog
-from datalab.gui.plugins.instrument import InstrumentWindow
+from datalab.gui.plugins.instrument import (
+    InstrumentWindow,
+    SerialWorker,
+    wait_instruments_closed,
+)
 from datalab.gui.processor.base import FeatureNotFoundError
 from datalab.gui.processor.preview import PreviewExecutorCache
 from datalab.gui.settings import AI_OPTION_NAMES, edit_settings
@@ -172,6 +177,10 @@ def __init__(self, console=None, hide_on_close=False):
         self.aiassistantpanel = None  # type: ignore[assignment]
         self.__applications_dialog: ApplicationsDialog | None = None
         self.__instrument_windows: dict[tuple[str, str], InstrumentWindow] = {}
+        #: Worker of each plugin instrument, shared by its successive windows
+        self.__instrument_workers: dict[tuple[str, str], SerialWorker] = {}
+        #: Title and closing call of the last closed window of each instrument
+        self.__instrument_closings: dict[tuple[str, str], tuple[str, Future]] = {}
         self.welcomepanel: WelcomePanel | None = None
 
         self.signalpanel_toolbar: QW.QToolBar | None = None
@@ -509,15 +518,18 @@ def add_group(
         title: str,
         panel: Literal["signal", "image"] | None = None,
         select: bool = False,
-    ) -> None:
+    ) -> str:
         """Add group to DataLab.
 
         Args:
             title: Group title
             panel: Panel name. Defaults to None.
             select: Select the group after creation. Defaults to False.
+
+        Returns:
+            UUID of the created group
         """
-        self.__get_datapanel(panel).add_group(title, select)
+        return get_uuid(self.__get_datapanel(panel).add_group(title, select))
 
     @remote_controlled
     def select_objects(
@@ -674,7 +686,9 @@ def call_method_slot(
             self.remote_server.exception = exc
 
     @remote_controlled
-    def run_remote_command(self, command: Callable[..., object], *args) -> None:
+    def run_remote_command(
+        self, command: Callable[..., object], *args, return_result: bool = False
+    ) -> None:
         """Slot to run a command requested by a remote client in GUI thread.
 
         The exception raised by the command, if any, is stored in RemoteServer to
@@ -684,9 +698,13 @@ def run_remote_command(self, command: Callable[..., object], *args) -> None:
         Args:
             command: Main window method running the command
             args: Command arguments
+            return_result: If True, store the command result in RemoteServer, to be
+             returned to the client (it must be XML-RPC serializable)
         """
         try:
-            command(*args)
+            result = command(*args)
+            if return_result:
+                self.remote_server.result = result
         except Exception as exc:  # pylint: disable=broad-except
             self.remote_server.exception = exc
 
@@ -1143,11 +1161,24 @@ def __launch_plugin_tool(self, plugin: PluginBase, tool_id: str) -> None:
     def open_plugin_instrument(
         self, plugin: PluginBase, tool: PluginTool
     ) -> InstrumentWindow:
-        """Show the instrument window of a plugin tool, creating it if needed."""
+        """Show the instrument window of a plugin tool, creating it if needed.
+
+        The successive windows of an instrument share its worker: a window
+        opened again calls the instrument once the closed one has closed it.
+        """
         key = (plugin.plugin_id, tool.id)
         window = self.__instrument_windows.get(key)
+        if window is not None and window.close_future is not None:
+            # Closed while hidden: Qt did not emit `finished`, release it now
+            self.__forget_instrument_window(key)
+            window = None
         if window is None:
-            window = InstrumentWindow(self, plugin.get_instrument(tool.id), tool.title)
+            worker = self.__instrument_workers.get(key)
+            if worker is None:
+                worker = self.__instrument_workers[key] = SerialWorker()
+            window = InstrumentWindow(
+                self, plugin.get_instrument(tool.id), tool.title, worker
+            )
             window.finished.connect(
                 functools.partial(self.__forget_instrument_window, key)
             )
@@ -1158,15 +1189,33 @@ def open_plugin_instrument(
         return window
 
     def __forget_instrument_window(self, key: tuple[str, str], *_args) -> None:
-        """Release a closed instrument window."""
+        """Release a closed instrument window, keeping track of its instrument
+        until it is closed."""
         window = self.__instrument_windows.pop(key, None)
         if window is not None:
+            window.stop()
+            closing = (window.windowTitle(), window.close_future)
+            self.__instrument_closings[key] = closing
             window.deleteLater()
 
     def close_plugin_instruments(self) -> None:
-        """Close all plugin instrument windows."""
+        """Close all plugin instrument windows, and wait for their instruments,
+        including those of windows closed before, within a common time limit."""
         for window in list(self.__instrument_windows.values()):
             window.close()
+        # Windows that were not visible do not emit `finished` when closed
+        for key in list(self.__instrument_windows):
+            self.__forget_instrument_window(key)
+        timeout = InstrumentWindow.CLOSE_TIMEOUT
+        closings = self.__instrument_closings.values()
+        for title in wait_instruments_closed(closings, timeout):
+            logging.getLogger(__name__).warning(
+                "Instrument '%s' did not close within %g s", title, timeout
+            )
+        for worker in self.__instrument_workers.values():
+            worker.shutdown()
+        self.__instrument_workers.clear()
+        self.__instrument_closings.clear()
 
     def __unregister_plugins(self) -> None:
         """Unregister all plugins and let them cleanup their hooks"""
@@ -2272,7 +2321,10 @@ def open_h5_files(
                     self,
                     _("Open"),
                     Conf.base_dir.get(),
-                    _("HDF5 files (*.h5 *.hdf5 *.hdf *.he5);;All files (*)"),
+                    _(
+                        "HDF5 files (*.h5 *.hdf5 *.hdf *.he5 *.nxs *.nx5);;"
+                        "All files (*)"
+                    ),
                 )
             if not h5files:
                 return

datalab/gui/panel/base.py

@@ -185,7 +185,9 @@ def is_hdf5_file(filename: str, check_content: bool = False) -> bool:
         True if the file is (likely) an HDF5 file, False otherwise.
     """
     # First, check by extension (fast)
-    has_hdf5_extension = filename.lower().endswith((".h5", ".hdf5", ".hdf", ".he5"))
+    has_hdf5_extension = filename.lower().endswith(
+        (".h5", ".hdf5", ".hdf", ".he5", ".nxs", ".nx5")
+    )
 
     if not check_content:
         return has_hdf5_extension