diff --git a/docs/source/getting_started.rst b/docs/source/getting_started.rst index f937dedc..4e755fc4 100644 --- a/docs/source/getting_started.rst +++ b/docs/source/getting_started.rst @@ -95,18 +95,17 @@ You can :doc:`view the results ` using the synchronized napari layers if you select the new labels/points layer as input: our next major feature to add is incorporating the detection and linking corrections into the optimization task in a more principled manner. -Each ``Tracking Run`` will be stored in the ``Tracks List`` widget. +Each ``Tracking Run`` will be stored in the ``Results List`` widget. These are the runs that are stored in memory - if you run tracking multiple times with different inputs or parameters, you can click back and forth -between the results here. Here you can also save any runs that you want to store for later, -or export the tracks to `geff`_ or csv, optionally including the (relabeled) segmentation. +between the results here. If your input was a Labels layer, the ``node_id`` will be determined by segmentation label id. If your original segmentation repeated labels across time, the application will relabel them all to be unique, and the new label id will be used as the node id. If your input was a Points layer, the ``node_id`` is simply the index of the node in the list of points. Deleting runs you do not want to keep viewing is a good idea, since these are stored in memory. -Runs that were saved in previous sessions do not appear here until you load them from disk with the ``Load Tracks`` button. +Tracks that were saved in previous sessions do not appear here until you load them from disk with the ``Load`` button. The tracking results can also be visualized as a lineage tree. You can open the lineage tree widget via ``Plugins`` > ``Motile Tracker`` > ``Widget - Lineage View``. For more details, go to the :doc:`Tree View ` documentation. @@ -116,6 +115,75 @@ Displaying feature measurements If you are tracking with a segmentation layer, you can select size and shape features to measure in the ``Features`` widget. Once selected, the measurements for these features will appear in the ``Lineage View`` (select ``Plot`` > ``Feature`` to display them), and in the ``Table`` widget. +.. _save-load-vs-import-export: + +Saving and loading vs. importing and exporting +********************************************** +The ``Results List`` widget offers two different ways of getting tracks in and out +of the application, and it is worth understanding which one you want. + +**Saving and loading** is for continuing your own work. The application controls the +format, so it can make and enforce assumptions about it: tracks are always written +as a `geff`_ store, with the metadata and attributes the application needs already +in place. Anything you save can be loaded back into a later session and picked up +exactly where you left off, including run-specific extras like the motile solver +parameters. Use this while you are still working on a dataset. + +**Importing and exporting** is for exchanging tracks with other tools. Here the +application cannot assume much about the format, so it supports more of them (`geff`_ +and CSV) and asks you to fill in the gaps - which column means what, how the data is +scaled, where the segmentation lives. An export is a snapshot for another tool to read, +not a session you can resume, and importing tracks from elsewhere requires the +column mapping step described in :doc:`Importing externally generated tracks `. + +In short: save/load round-trips within the application, import/export crosses the +boundary to other tools. + +Saving tracks +------------- +Above the results list are a ``Save directory`` field, with a ``Browse`` button, and a +``Save filename`` field. Together these are the path that the save (floppy disk) button +beside a set of tracks writes to; the ``.geff`` suffix is added for you and shown as a +fixed label beside the filename. The directory starts out as an application-owned +location (the same place the sample data is downloaded to) and the filename follows +whichever tracks you have selected, so in the common case you can simply click save. + +Both fields are editable, and your edits last for the rest of the session - so if you +point the directory somewhere else once, subsequent saves go there too. Once you have +typed your own filename it stops following the selection, so selecting different tracks +will not overwrite what you typed. Because names in the results list are not required to +be unique, always check the filename before saving if you have several similarly named +sets of tracks. + +Saving writes to exactly the path shown; there is no timestamped subdirectory. If +something already exists at that path you will be asked to confirm before it is +replaced. Note that saving a set of tracks over an existing geff store replaces the +tracks but leaves any other files in the store alone. + +Loading tracks +-------------- +The dropdown menu at the bottom of the widget selects what to load, and the ``Load`` +button starts it: + +- ``Tracks (geff)`` - load tracks previously saved from this application. Select the + ``.geff`` store itself. +- ``Motile Run`` - load a saved motile run, which restores the solver parameters into + the ``Run Editor`` along with the tracks. Select the ``.geff`` store the run was + saved to. Runs saved by older versions, which used a timestamped directory + containing the tracks and a separate parameters file, can still be loaded. +- ``External tracks from CSV`` and ``External tracks from geff`` - import tracks that + were generated elsewhere. These open the import dialog, where you map columns to + attributes and optionally provide a segmentation; see + :doc:`Importing externally generated tracks `. + +Exporting tracks +---------------- +The export button beside a set of tracks in the results list opens the export dialog, +where you choose ``GEFF`` +or ``CSV`` and pick the location, optionally including the (relabeled) segmentation as +zarr or tiff. You can also export a subset of tracks from the Groups tab. Exported tracks are meant to be read by other tools: to continue working +on them here later, save them instead. + .. _Issue #48: https://github.com/funkelab/motile_tracker/issues/48 .. _Cell Tracking Challenge: https://celltrackingchallenge.net/ .. _Fabrèges et al (2024): https://www.science.org/doi/10.1126/science.adh1145 diff --git a/docs/source/view_external_tracks.rst b/docs/source/view_external_tracks.rst index 3a9ec6e1..8714de35 100644 --- a/docs/source/view_external_tracks.rst +++ b/docs/source/view_external_tracks.rst @@ -1,15 +1,25 @@ -Viewing Externally Generated Tracks -=================================== +Importing Externally Generated Tracks +===================================== Usage Overview ************** It is also possible to view tracks that were not created from the motile widget using -the synchronized Lineage View and napari layers. To do so, navigate to the ``Tracks List`` tab and +the synchronized Lineage View and napari layers. Bringing them in is an *import* rather +than a load: because the application cannot assume anything about how another tool +wrote the data, you have to describe its layout as part of importing it. See +:ref:`save-load-vs-import-export` for how this differs from loading tracks you saved +here yourself. + +To import, navigate to the ``Results List`` tab and select ``External tracks from CSV`` or ``External tracks from geff`` in the dropdown menu at the bottom of the widgets, and click ``Load``. A pop up menu will allow you to select a CSV file or geff zarr folder and map its columns to the required default attributes and optional additional attributes. You may also provide the accompanying segmentation and specify scaling information. +Once imported, tracks appear in the results list like any other set of tracks, and can +be edited and then saved in the application's own format so that later sessions can load +them back without repeating the column mapping. + The following columns have to be selected: - time: representing the position of the object in the time dimension. diff --git a/pyproject.toml b/pyproject.toml index 21905eaf..a749d151 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ classifiers = [ dependencies =[ "napari>=0.6.2,<0.8.0", - "funtracks>=2,<3", + "funtracks>=2.0.3,<3", "appdirs>=1,<2", "numpy>=2,<3", "magicgui>=0.10.1", @@ -109,7 +109,11 @@ addopts = "--ignore=tests/benchmarks" # that pointing pytest at the tests/benchmarks directory discovers them. python_files = ["test_*.py", "bench_*.py"] filterwarnings = [ - "error::DeprecationWarning:funtracks", + # Temporarily disabled: motile_tracker still converts to the deprecated + # SolutionTracks when emitting tracks, because the views/actions require + # track IDs. Re-add when we refactor to operate on Tracks directly, ahead + # of the next major funtracks release. + # "error::DeprecationWarning:funtracks", ] [tool.setuptools_scm] diff --git a/src/motile_tracker/data_views/views_coordinator/tracks_list.py b/src/motile_tracker/data_views/views_coordinator/tracks_list.py index 717dfaae..ddb69a6b 100644 --- a/src/motile_tracker/data_views/views_coordinator/tracks_list.py +++ b/src/motile_tracker/data_views/views_coordinator/tracks_list.py @@ -1,9 +1,14 @@ +from __future__ import annotations + +from collections.abc import Callable from functools import partial from pathlib import Path from warnings import warn +from appdirs import AppDirs from fonticon_fa6 import FA6S -from funtracks.data_model import Tracks +from funtracks.data_model import SolutionTracks, Tracks +from funtracks.import_export import import_from_geff from napari._qt.qt_resources import QColoredSVGIcon from qtpy.QtCore import Signal from qtpy.QtWidgets import ( @@ -14,20 +19,76 @@ QGroupBox, QHBoxLayout, QLabel, + QLineEdit, QListWidget, QListWidgetItem, + QMessageBox, QPushButton, QVBoxLayout, QWidget, ) from superqt.fonticon import icon as qticon +from motile_tracker.import_export.geff_io import write_geff_over from motile_tracker.import_export.menus.export_dialog import ExportDialog from motile_tracker.import_export.menus.import_dialog import ( ImportDialog, ) from motile_tracker.motile.backend.motile_run import MotileRun +GEFF_SUFFIX = ".geff" + + +def default_save_dir() -> Path: + """Directory the save path starts in. + + The same appdirs location the sample data is downloaded to, so that saved + tracks land somewhere the application already owns rather than in the + user's home directory. + """ + return Path(AppDirs("motile-tracker").user_data_dir) + + +def _as_solution_tracks(tracks: Tracks) -> SolutionTracks: + """Return a SolutionTracks view of the given tracks. + + The list stores plain Tracks, but the views and actions downstream of + view_tracks still require track IDs, so they are handed a SolutionTracks. + Objects that are already SolutionTracks (including MotileRun) are passed + through unchanged, so a solved run keeps its solver params and identity. + + Constructs directly rather than using SolutionTracks.from_tracks, which in + funtracks 2.0.x reads features.tracklet_key off the graph before building + anything. tracklet_key only declares which attribute *would* hold the + tracklet id; whether it exists is a separate question (whether that key is + in the FeatureDict). Tracks whose tracklet column was never created + therefore raise KeyError there. + + TODO: remove once motile_tracker operates on Tracks directly and consumers + call tracks.graph_solution themselves. + """ + if isinstance(tracks, SolutionTracks): + return tracks + solution_tracks = SolutionTracks( + tracks.graph, + scale=tracks.scale, + ndim=tracks.ndim, + features=tracks.features, + _segmentation=tracks.segmentation, + ) + # Only needed on funtracks < 2.1, where passing a FeatureDict makes + # __init__ activate the declared features without computing the missing + # ones. From 2.1 every Tracks already has track ids, so this finds nothing. + features = solution_tracks.features + missing = [ + key + for key in (features.tracklet_key, features.lineage_key) + if key is not None and key not in solution_tracks.graph.node_attr_keys() + ] + if missing: + solution_tracks.enable_features(missing) + return solution_tracks + class TracksButton(QWidget): # https://doc.qt.io/qt-5/qlistwidget.html#setItemWidget @@ -74,6 +135,30 @@ class TracksList(QGroupBox): view_tracks = Signal(Tracks, str) request_colormap = Signal() + tracks_saved = Signal(object, Path) + """Emitted after tracks are saved to disk. Arguments: (tracks, path). + Dependent applications can connect to this signal to save additional + data (e.g. solver parameters) alongside the tracks. + + The path is the geff store the tracks were written to. A geff is a zarr + directory, and writing one only replaces geff-controlled groups, so + listeners should write their own data *inside* this path: it survives the + tracks being saved again over the same store.""" + + tracks_loaded = Signal(object, Path) + """Emitted after tracks are loaded from disk. Arguments: (tracks, path). + Dependent applications can connect to this signal to load additional + data (e.g. solver parameters) from the same location. + + The path is the geff store the tracks were read from, matching what + tracks_saved reports for the same tracks, so data written inside it by a + tracks_saved listener is found here. It is never a container a geff merely + happened to be found inside. + + The exceptions are a CSV import, which reports the .csv file, and a v1 motile + run directory, which reports the directory holding the networkx graph json. Listeners + should tolerate a file as well as a directory.""" + def __init__(self): super().__init__(title="Results List") @@ -82,9 +167,32 @@ def __init__(self): self.file_dialog.setFileMode(QFileDialog.Directory) self.file_dialog.setOption(QFileDialog.ShowDirsOnly, True) - self.save_dialog = QFileDialog() - self.save_dialog.setFileMode(QFileDialog.Directory) - self.save_dialog.setOption(QFileDialog.ShowDirsOnly, True) + # Where the save button writes to. Auto-filled, but the user can point + # it anywhere; edits last for the session. The label and browse button + # sit above the directory field so that the path, which is long, gets + # the full width of the dock. + self._save_name_edited = False + + self.save_browse_button = QPushButton("Browse") + self.save_browse_button.setAutoDefault(0) + self.save_browse_button.clicked.connect(self._browse_save_dir) + + save_dir_header = QHBoxLayout() + save_dir_header.addWidget(QLabel("Save directory:")) + save_dir_header.addStretch() + save_dir_header.addWidget(self.save_browse_button) + + self.save_dir_line = QLineEdit(str(default_save_dir())) + + # The .geff suffix is shown as a fixed label rather than being typed, + # so the user cannot omit or misspell it. save_path() adds it back. + self.save_name_line = QLineEdit() + self.save_name_line.textEdited.connect(self._on_save_name_edited) + + save_name_row = QHBoxLayout() + save_name_row.addWidget(QLabel("Save filename:")) + save_name_row.addWidget(self.save_name_line) + save_name_row.addWidget(QLabel(GEFF_SUFFIX)) self.tracks_list = QListWidget() self.tracks_list.setSelectionMode( @@ -95,7 +203,12 @@ def __init__(self): load_menu = QHBoxLayout() self.dropdown_menu = QComboBox() self.dropdown_menu.addItems( - ["Motile Run", "External tracks from CSV", "External tracks from geff"] + [ + "Tracks (geff)", + "Motile Run", + "External tracks from CSV", + "External tracks from geff", + ] ) load_button = QPushButton("Load") @@ -105,33 +218,96 @@ def __init__(self): load_menu.addWidget(load_button) layout = QVBoxLayout() + layout.addLayout(save_dir_header) + layout.addWidget(self.save_dir_line) + layout.addLayout(save_name_row) layout.addWidget(self.tracks_list) layout.addLayout(load_menu) self.setLayout(layout) - def _load_tracks(self, import_type: str): + def _load_tracks(self, import_type: str) -> tuple[Tracks, str, Path | None] | None: + """Load externally generated tracks (CSV or geff) via the import dialog. + + Returns (tracks, name, path), where the path may be None because the + import dialog does not always know the file the tracks came from. + """ dialog = ImportDialog(import_type) - if dialog.exec_() == QDialog.Accepted: - tracks = dialog.tracks - name = dialog.name - if tracks is not None: - self.add_tracks(tracks, name, select=True) + if dialog.exec_() != QDialog.Accepted or dialog.tracks is None: + return None + return dialog.tracks, dialog.name, dialog.source_path + + def _browse_save_dir(self) -> None: + """Let the user pick the directory that saved tracks are written to.""" + directory = QFileDialog.getExistingDirectory( + self, "Select save directory", self.save_dir_line.text() + ) + if directory: + self.save_dir_line.setText(directory) + + def _confirm_overwrite(self, path: Path) -> bool: + """Ask before replacing something already at the save path. + + Saving no longer goes through a file dialog, so this is the only thing + standing between a stray click and an overwritten store. + """ + answer = QMessageBox.question( + self, + "Replace existing tracks?", + f"{path} already exists. Replace it?", + QMessageBox.Yes | QMessageBox.No, + QMessageBox.No, + ) + return answer == QMessageBox.Yes + + def _on_save_name_edited(self) -> None: + """Stop auto-filling the name once the user has typed their own. + + Connected to textEdited rather than textChanged, so that the + programmatic setText in _update_save_name does not count as an edit. + """ + self._save_name_edited = True + + def _update_save_name(self, name: str) -> None: + """Point the save name at the given tracks, unless the user renamed it. + + Names in the list are not unique, so this only ever changes the name + field: the directory stays put, and a collision shows up as a visible + name the user can edit rather than silently redirecting the save. + + The name is shown without its .geff suffix, which the UI displays as a + fixed label beside the field. + """ + if not self._save_name_edited: + self.save_name_line.setText(name.removesuffix(GEFF_SUFFIX)) + + def save_path(self) -> Path | None: + """The geff store that the save button writes to. + + Combines the save directory with the filename and re-attaches the + .geff suffix that the UI shows as a fixed label. Returns None if + either field is blank. + """ + directory = self.save_dir_line.text().strip() + name = self.save_name_line.text().strip().removesuffix(GEFF_SUFFIX) + if not directory or not name: + return None + return Path(directory) / f"{name}{GEFF_SUFFIX}" def _selection_changed(self): selected = self.tracks_list.selectedItems() if selected: tracks_button = self.tracks_list.itemWidget(selected[0]) - self.view_tracks.emit(tracks_button.tracks, tracks_button.name.text()) + name = tracks_button.name.text() + self._update_save_name(name) + self.view_tracks.emit(_as_solution_tracks(tracks_button.tracks), name) def add_tracks(self, tracks: Tracks, name: str, select=True): - """Add a run to the list and optionally select it. Will make a new - row in the list UI representing the given run. + """Add tracks to the list and optionally select them. Will make a new + row in the list UI representing the given tracks. - Accepts any Tracks object. Plain Tracks/SolutionTracks are wrapped in - a MotileRun (with solver_params=None) so the list internally always - holds MotileRun and save_tracks can rely on tracks.save(). + Accepts any Tracks object directly (SolutionTracks, MotileRun, etc.). - Note: selecting the run will also emit the selection changed event on + Note: selecting the tracks will also emit the selection changed event on the list. Args: @@ -140,18 +316,6 @@ def add_tracks(self, tracks: Tracks, name: str, select=True): select (bool, optional): Whether or not to select the new tracks item in the list (and thus display it in the tracks viewer). Defaults to True. """ - if not isinstance(tracks, MotileRun): - tracks = MotileRun( - graph=tracks.graph, - run_name=name, - solver_params=None, - pos_attr=tracks.features.position_key, - time_attr=tracks.features.time_key, - scale=tracks.scale, - ndim=tracks.ndim, - _features=tracks.features, - _segmentation=tracks.segmentation, - ) item = QListWidgetItem(self.tracks_list) tracks_row = TracksButton(tracks, name) self.tracks_list.setItemWidget(item, tracks_row) @@ -188,14 +352,36 @@ def save_tracks(self, item: QListWidgetItem): """Saves a tracks object from the list. You must pass the list item that represents the tracks, not the tracks object itself. + Writes a geff store at the path shown in the save fields above the + list, confirming first if something is already there. A MotileRun + additionally stores its solver params inside that store. + + After saving, emits the tracks_saved signal with the geff store that + was written, so that downstream code can save additional data inside + it. + Args: item (QListWidgetItem): The list item to save. This list item contains the TracksButton that represents a set of tracks. """ - tracks: Tracks = self.tracks_list.itemWidget(item).tracks - if self.save_dialog.exec_(): - directory = Path(self.save_dialog.selectedFiles()[0]) - tracks.save(directory) + saved_path = self.save_path() + if saved_path is None: + warn( + "Cannot save without both a save directory and a filename.", + stacklevel=2, + ) + return + if saved_path.exists() and not self._confirm_overwrite(saved_path): + return + + widget: TracksButton = self.tracks_list.itemWidget(item) + tracks: Tracks = widget.tracks + saved_path.parent.mkdir(parents=True, exist_ok=True) + if isinstance(tracks, MotileRun): + tracks.save(saved_path) + else: + write_geff_over(tracks, saved_path) + self.tracks_saved.emit(tracks, saved_path) def remove_tracks(self, item: QListWidgetItem): """Remove a tracks object from the list. You must pass the list item that @@ -209,27 +395,66 @@ def remove_tracks(self, item: QListWidgetItem): self.tracks_list.takeItem(row) def load_tracks(self): - """Call the function to load tracks from disk for a Motile Run or for externally - generated tracks (CSV file), depending on the choice in the dropdown menu. - """ - - if self.dropdown_menu.currentText() == "Motile Run": - self.load_motile_run() - elif self.dropdown_menu.currentText() == "External tracks from CSV": - self._load_tracks(import_type="csv") - elif self.dropdown_menu.currentText() == "External tracks from geff": - self._load_tracks("geff") + """Load tracks from disk, depending on the choice in the dropdown menu. - def load_motile_run(self): - """Load a set of tracks from disk. The user selects the directory created - by calling save_tracks. + Each loader returns the loaded tracks along with the name to display and + the path they came from, or None if the user cancelled or the load + failed. Adding the tracks to the list and announcing them via + tracks_loaded happens here, so every load route behaves the same way. """ + selection = self.dropdown_menu.currentText() + if selection == "Tracks (geff)": + result = self.load_internal_tracks() + elif selection == "Motile Run": + result = self.load_motile_run() + elif selection == "External tracks from CSV": + result = self._load_tracks(import_type="csv") + elif selection == "External tracks from geff": + result = self._load_tracks("geff") + else: + return + + if result is None: + return + tracks, name, source_path = result + self.add_tracks(tracks, name, select=True) + if source_path is not None: + self.tracks_loaded.emit(tracks, source_path) + + def _load_from_dialog( + self, + loader: Callable[[Path], Tracks], + geff_path: Callable[[Path], Path | None] | None = None, + ) -> tuple[Tracks, str, Path] | None: + """Ask the user for a directory and load tracks from it with `loader`. + + The name shown in the list comes from the directory the user picked. The + reported path is the geff store that was actually read, which `geff_path` + resolves when the user picks a directory containing one rather than the + store itself. + + Returns (tracks, name, path), or None if the user cancelled or the + directory did not contain loadable tracks. + """ + if not self.file_dialog.exec_(): + return None + directory = Path(self.file_dialog.selectedFiles()[0]) + try: + tracks = loader(directory) + except (ValueError, FileNotFoundError) as e: + warn(f"Could not load tracks from {directory}: {e}", stacklevel=2) + return None + source = directory if geff_path is None else geff_path(directory) + return tracks, directory.stem, source or directory + + def load_internal_tracks(self) -> tuple[Tracks, str, Path] | None: + """Load tracks saved in internal format. The user selects the GEFF + store directly (the path written by :func:`write_to_geff`). + """ + return self._load_from_dialog(import_from_geff) - if self.file_dialog.exec_(): - directory = Path(self.file_dialog.selectedFiles()[0]) - name = directory.stem - try: - tracks = MotileRun.load(directory) - self.add_tracks(tracks, name, select=True) - except (ValueError, FileNotFoundError) as e: - warn(f"Could not load tracks from {directory}: {e}", stacklevel=2) + def load_motile_run(self) -> tuple[Tracks, str, Path] | None: + """Load a MotileRun from disk. The user selects the directory created + by MotileRun.save(), and the geff store inside it is reported. + """ + return self._load_from_dialog(MotileRun.load, geff_path=MotileRun.geff_path) diff --git a/src/motile_tracker/data_views/views_coordinator/tracks_viewer.py b/src/motile_tracker/data_views/views_coordinator/tracks_viewer.py index 1583d1b6..4d9b23f2 100644 --- a/src/motile_tracker/data_views/views_coordinator/tracks_viewer.py +++ b/src/motile_tracker/data_views/views_coordinator/tracks_viewer.py @@ -63,6 +63,12 @@ def get_instance(cls, viewer=None): ): if viewer is None: raise ValueError("Make a viewer first please!") + # The outgoing instance is about to become unreachable, but psygnal + # connections keep it subscribed to its tracks object. Unsubscribe it, + # or a tracks object shown in successive viewers ends up notifying + # every TracksViewer ever built (see _disconnect_tracks). + if hasattr(cls, "_instance"): + cls._instance._disconnect_tracks() cls._instance = TracksViewer(viewer) return cls._instance @@ -77,6 +83,7 @@ def __init__( self.table_widget_present = False def _clear_if_current(): + self._disconnect_tracks() if hasattr(TracksViewer, "_instance") and TracksViewer._instance is self: del TracksViewer._instance @@ -247,6 +254,20 @@ def _refresh(self, node: str | None = None, refresh_view: bool = False) -> None: # know about their selection ('all' vs 'lineage'), but TracksViewer does) self.update_selection(update_counts=True) + def _disconnect_tracks(self) -> None: + """Stop listening to the currently displayed tracks object. + + The connections below live on the Tracks object, not on this TracksViewer, + so they outlive both the viewer and the singleton reference unless they are + explicitly removed. Because one Tracks object can be handed to more than one + viewer over a session, leaving them in place means an edit notifies every + TracksViewer that ever displayed those tracks. + """ + tracks = getattr(self, "tracks", None) + if tracks is not None: + tracks.refresh.disconnect(self._refresh) + tracks.action_applied.disconnect(self._on_action_applied) + def update_tracks(self, tracks: SolutionTracks, name: str) -> None: """Stop viewing a previous set of tracks and replace it with a new one. Will create new segmentation and tracks layers and add them to the viewer. @@ -257,9 +278,7 @@ def update_tracks(self, tracks: SolutionTracks, name: str) -> None: """ self.selected_nodes.reset() - if self.tracks is not None: - self.tracks.refresh.disconnect(self._refresh) - self.tracks.action_applied.disconnect(self._on_action_applied) + self._disconnect_tracks() self.tracks = tracks self.selected_nodes.deleted_items.clear() # Reset deleted nodes when switching tracks diff --git a/src/motile_tracker/import_export/geff_io.py b/src/motile_tracker/import_export/geff_io.py new file mode 100644 index 00000000..b0e819f6 --- /dev/null +++ b/src/motile_tracker/import_export/geff_io.py @@ -0,0 +1,75 @@ +"""Helpers for writing geff stores that hold extra, non-geff files. + +Saving tracks writes a geff store, and dependent applications add their own +files inside it by listening to TracksList.tracks_saved. That is a supported +arrangement — a geff is a zarr directory, and rewriting the geff leaves +everything else alone — but it makes both zarr and geff chatty. +""" + +from __future__ import annotations + +import warnings +from pathlib import Path + +from funtracks.data_model import Tracks +from funtracks.import_export import write_to_geff + + +def is_geff(directory: Path) -> bool: + """Whether the given directory is itself a geff store. + + A geff keeps its graph in `nodes`/`edges` groups at the top level, so their + presence distinguishes a directory that is a geff from one that merely + contains one, or is empty. + + Note that geff's own `check_for_geff` cannot be used here: it reports + whether a geff exists at or under a store, and so returns True for an old + run directory containing tracks.geff, for a v1 run directory, and even for + an empty one. + """ + return (directory / "nodes").exists() and (directory / "edges").exists() + + +def write_geff_over(tracks: Tracks, path: Path) -> None: + """Write tracks to a geff store, replacing any geff already there. + + Saved tracks are a geff store that dependents may also keep their own files + in: tracks_saved listeners write extra data (e.g. solver params) inside the + store, and it survives because writing a geff only replaces geff-controlled + groups. Zarr walks the directory on the way and warns once per file it does + not recognise, and geff warns that it found non-geff members. Both are + expected for any store with extras in it and say nothing the caller can act + on, so they are silenced. + + The filters match on message rather than category: zarr only grew a + dedicated ZarrUserWarning class in 3.x, and this package supports 2.x, + where importing it fails outright. + + `overwrite` is only passed when there really is a geff to replace, because + geff deletes the old graph by removing its `nodes`/`edges` groups outright + and raises KeyError if they are absent. An empty directory needs more than + that: geff's `check_for_geff` reports one as an existing geff, so writing + without `overwrite` raises FileExistsError while writing with it raises + KeyError. Removing it first leaves geff to create the store itself. Only + an empty directory is removed, never one holding a caller's own files. + + Args: + tracks (Tracks): The tracks to write. + path (Path): The geff store to write them to. Created if it does not + exist; any geff already there is replaced. + """ + if path.is_dir() and not any(path.iterdir()): + path.rmdir() + + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + message="Object at .* is not recognized as a component of a Zarr hierarchy", + category=UserWarning, + ) + warnings.filterwarnings( + "ignore", + message="Found non-geff members in zarr.*", + category=UserWarning, + ) + write_to_geff(tracks, path, overwrite=is_geff(path)) diff --git a/src/motile_tracker/import_export/menus/import_dialog.py b/src/motile_tracker/import_export/menus/import_dialog.py index 7a335812..1de211b6 100644 --- a/src/motile_tracker/import_export/menus/import_dialog.py +++ b/src/motile_tracker/import_export/menus/import_dialog.py @@ -50,6 +50,7 @@ def __init__(self, import_type: str = "csv") -> None: self.seg = None self.df = None self.incl_z = False + self.source_path: Path | None = None self.setWindowTitle(f"Import external tracks from {import_type.upper()}") self.name = f"Tracks from {import_type.upper()}" @@ -379,6 +380,11 @@ def _finish(self) -> None: except Exception as e: # noqa: BLE001 QMessageBox.critical(self, "Error", f"Failed to load tracks: {e}") return + # Report the geff group we actually read, not the container it + # was found in: a listener uses this path to find data saved + # alongside the tracks, and the container may hold several + # groups. + self.source_path = geff_dir self.accept() else: if self.df is not None: @@ -407,4 +413,6 @@ def _finish(self) -> None: except Exception as e: # noqa: BLE001 QMessageBox.critical(self, "Error", f"Failed to load tracks: {e}") return + csv_text = self.import_widget.csv_path_line.text().strip() + self.source_path = Path(csv_text) if csv_text else None self.accept() diff --git a/src/motile_tracker/motile/backend/motile_run.py b/src/motile_tracker/motile/backend/motile_run.py index c54f282b..a2951c61 100644 --- a/src/motile_tracker/motile/backend/motile_run.py +++ b/src/motile_tracker/motile/backend/motile_run.py @@ -8,7 +8,9 @@ import numpy as np import tracksdata as td from funtracks.data_model import SolutionTracks -from funtracks.import_export import export_to_geff, import_from_geff, load_v1_tracks +from funtracks.import_export import import_from_geff, load_v1_tracks + +from motile_tracker.import_export.geff_io import is_geff, write_geff_over from .solver_params import SolverParams @@ -103,22 +105,57 @@ def _unpack_id(_id: str) -> tuple[datetime, str]: ) from e return time, run_name - def save(self, base_path: str | Path, save_segmentation: bool = False) -> Path: - """Save the run in the provided directory. Creates a subdirectory from - the timestamp and run name and stores one file for each element of the - run in that subdirectory. + @classmethod + def _resolve_name_and_time( + cls, run_dir: Path, attrs: dict | None + ) -> tuple[datetime | None, str]: + """Determine the run name and run time for a run being loaded. + + Runs used to be saved in a directory named by _make_id, so the name and + time could be recovered by unpacking the directory name. Newer runs + store both in the attrs file instead, which lets them be saved to a + directory the user named. Falls back through both, and finally to the + directory name with no time, so that a run directory is loadable + however it was named. A None time is replaced with the current time by + __init__, so the run still displays. Args: - base_path (str | Path): The directory to save the run in. + run_dir (Path): The directory the run is being loaded from. + attrs (dict | None): The loaded attrs, or None if there is no + attrs file. Returns: - (Path): The Path that the run was saved in. The last part of the - path is the directory that was created to store the run. + tuple[datetime | None, str]: The run time and run name. """ - base_path = Path(base_path) - run_dir = base_path / self._make_id() - Path.mkdir(run_dir) - export_to_geff(self, run_dir, save_segmentation=save_segmentation) + if attrs is not None and attrs.get("run_name") is not None: + stamp = attrs.get("time") + time = datetime.fromisoformat(stamp) if stamp is not None else None + return time, attrs["run_name"] + try: + return cls._unpack_id(run_dir.stem) + except ValueError: + return None, run_dir.stem + + def save(self, path: str | Path, save_segmentation: bool = False) -> Path: + """Save the run as a geff store at the provided path. + + The geff store is written at exactly `path` — no subdirectory is + created — and the rest of the run (solver params, attrs, input points, + gaps) is stored inside that store alongside the graph. A geff is a zarr + directory, and writing a geff only replaces geff-controlled groups, so + these files survive re-saving over the same store. + + Args: + path (str | Path): The geff store to save the run to. Created if + it does not exist, and replaced if it does. + save_segmentation (bool): Ignored. Kept for backwards + compatibility; the segmentation is never written here. + + Returns: + (Path): The Path that the run was saved to. + """ + run_dir = Path(path) + write_geff_over(self, run_dir) self._save_params(run_dir) self._save_attrs(run_dir) if self.input_points is not None: @@ -126,6 +163,36 @@ def save(self, base_path: str | Path, save_segmentation: bool = False) -> Path: self._save_list(list_to_save=self.gaps, run_dir=run_dir, filename=GAPS_FILENAME) return run_dir + @staticmethod + def geff_path(run_dir: Path | str) -> Path | None: + """Return the geff store holding a saved run's graph. + + Mirrors the layouts that :meth:`load` accepts. Runs saved by the + current version are themselves the geff store. Returns None for v1 + runs, which stored the graph as graph.json rather than as a geff. + + Args: + run_dir (Path | str): A directory created by MotileRun.save. + """ + run_dir = Path(run_dir) + if MotileRun._is_geff(run_dir): + return run_dir + tracks_path = run_dir / "tracks.geff" + if tracks_path.exists(): + return tracks_path + if (run_dir / "graph.json").exists(): + return None + return run_dir / "tracks" + + @staticmethod + def _is_geff(directory: Path) -> bool: + """Whether the given directory is itself a geff store. + + Distinguishes a run saved as a geff from an older run directory that + merely contains one, which is exactly what load() needs. + """ + return is_geff(directory) + @classmethod def load(cls, run_dir: Path | str, output_required: bool = True): """Load a run from disk into memory. @@ -143,14 +210,17 @@ def load(cls, run_dir: Path | str, output_required: bool = True): """ if isinstance(run_dir, str): run_dir = Path(run_dir) - time, run_name = cls._unpack_id(run_dir.stem) params = cls._load_params(run_dir) input_points = cls._load_array(run_dir, IN_POINTS_FILENAME, required=False) attrs = cls._load_attrs(run_dir) - # Support old v1 ("graph.json" at run dir level), intermediate ("tracks" zarr), - # and new ("tracks.geff") save formats + time, run_name = cls._resolve_name_and_time(run_dir, attrs) + # Support the current format (the run dir is itself the geff store) as + # well as old v1 ("graph.json" at run dir level), intermediate + # ("tracks" zarr), and ("tracks.geff") save formats tracks_path = run_dir / "tracks.geff" - if tracks_path.exists(): + if cls._is_geff(run_dir): + tracks = import_from_geff(run_dir) + elif tracks_path.exists(): tracks = import_from_geff(tracks_path) elif (run_dir / "graph.json").exists(): tracks = load_v1_tracks(run_dir, solution=True) @@ -184,8 +254,8 @@ def load(cls, run_dir: Path | str, output_required: bool = True): def _save_params(self, run_dir: Path): """Save the run parameters in the provided run directory. Currently dumps the parameters dict into a json file. Skips writing if there are - no params (e.g. tracks imported from CSV/geff that never went through - the solver). + no params, which only happens for a run loaded from a directory that + had no params file (see _load_params). Args: run_dir (Path): A directory in which to save the parameters file. @@ -199,8 +269,9 @@ def _save_params(self, run_dir: Path): @staticmethod def _load_params(run_dir: Path) -> SolverParams | None: """Load parameters from the parameters json file in the provided - directory. Returns None if the file is absent — runs imported from - CSV/geff are saved without solver params. + directory. Returns None if the file is absent, which is the case for + v1 run directories and for runs saved by versions that wrapped + imported (CSV/geff) tracks in a MotileRun with no solver params. Args: run_dir (Path): The directory in which to find the parameters file. @@ -259,7 +330,15 @@ def _load_array( return None def _save_attrs(self, directory: Path): - """Save the time_attr, pos_attr, scale, and segmentation_shape in a json file. + """Save the run name, run time, time_attr, pos_attr, scale, and + segmentation_shape in a json file. + + The run name and time are stored here rather than being recoverable + from the directory name alone (see _make_id), so that a run can be + saved to a directory the user named. + + Note that "time" is when the run was solved, while "time_attr" is the + name of the graph's time column. Args: directory (Path): The directory in which to save the attributes @@ -275,6 +354,8 @@ def _save_attrs(self, directory: Path): "segmentation_shape": list(seg_shape) if seg_shape is not None else None, "scale": scale, "time_attr": self.features.time_key, + "run_name": self.run_name, + "time": self.time.isoformat(), } with open(out_path, "w") as f: json.dump(attrs_dict, f) diff --git a/src/motile_tracker/motile/menus/run_editor.py b/src/motile_tracker/motile/menus/run_editor.py index 691a044c..da856d05 100644 --- a/src/motile_tracker/motile/menus/run_editor.py +++ b/src/motile_tracker/motile/menus/run_editor.py @@ -231,8 +231,9 @@ def emit_run(self) -> None: def new_run(self, run: MotileRun) -> None: """Configure the run editor to copy the name and params of the given - run. Imported runs (CSV/geff) have no solver_params — leave the - editor at its current values rather than emitting None. + run. A run loaded from a directory with no params file has no + solver_params — leave the editor at its current values rather than + emitting None. """ self.run_name.setText(run.run_name) if run.solver_params is not None: diff --git a/src/motile_tracker/motile/menus/run_viewer.py b/src/motile_tracker/motile/menus/run_viewer.py index d20e459d..1a6fad03 100644 --- a/src/motile_tracker/motile/menus/run_viewer.py +++ b/src/motile_tracker/motile/menus/run_viewer.py @@ -55,8 +55,9 @@ def update_run(self, run: MotileRun): run_name_view = f"{run.run_name} ({run_time})" self.setTitle("Run Viewer: " + run_name_view) self.solver_event_update() - # Imported runs (CSV/geff) have no solver_params — hide the params - # display rather than emit None into widgets that can't render it. + # A run loaded from a directory with no params file has no + # solver_params — hide the params display rather than emit None into + # widgets that can't render it. if run.solver_params is None: self.params_widget.hide() else: diff --git a/tests/data_views/views_coordinator/test_tracks_list.py b/tests/data_views/views_coordinator/test_tracks_list.py index 2e1394dc..cd145734 100644 --- a/tests/data_views/views_coordinator/test_tracks_list.py +++ b/tests/data_views/views_coordinator/test_tracks_list.py @@ -1,18 +1,21 @@ """Tests for TracksList and TracksButton. -Covers add/remove/select tracks, save/load/export dialogs, and the -load_motile_run bug fix (must call MotileRun.load, not Tracks.load). +Covers add/remove/select tracks, save/load/export dialogs, signal emission, +and the load_motile_run bug fix (must call MotileRun.load, not Tracks.load). """ import warnings from unittest.mock import MagicMock, patch import pytest +from funtracks.data_model import SolutionTracks, Tracks +from funtracks.import_export import write_to_geff from qtpy.QtWidgets import QDialog from motile_tracker.data_views.views_coordinator.tracks_list import ( TracksButton, TracksList, + default_save_dir, ) from motile_tracker.motile.backend.motile_run import MotileRun, SolverParams @@ -24,7 +27,7 @@ def clear_viewer_layers(viewer): @pytest.fixture -def tracks(graph_2d): +def motile_run(graph_2d): return MotileRun(graph=graph_2d, run_name="test", solver_params=SolverParams()) @@ -39,13 +42,13 @@ def tracks_list(): class TestTracksButton: - def test_init_stores_tracks_and_name(self, tracks): - btn = TracksButton(tracks, "my_run") - assert btn.tracks is tracks + def test_init_stores_tracks_and_name(self, motile_run): + btn = TracksButton(motile_run, "my_run") + assert btn.tracks is motile_run assert btn.name.text() == "my_run" - def test_size_hint_height(self, tracks): - btn = TracksButton(tracks, "my_run") + def test_size_hint_height(self, motile_run): + btn = TracksButton(motile_run, "my_run") assert btn.sizeHint().height() == 30 @@ -55,33 +58,204 @@ def test_size_hint_height(self, tracks): class TestTracksListAddRemove: - def test_add_tracks_appends_item(self, tracks_list, tracks): - tracks_list.add_tracks(tracks, "run1", select=False) + def test_add_tracks_appends_item(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=False) assert tracks_list.tracks_list.count() == 1 - def test_add_tracks_with_select(self, tracks_list, tracks): - tracks_list.add_tracks(tracks, "run1", select=True) + def test_add_tracks_with_select(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=True) assert tracks_list.tracks_list.currentRow() == 0 - def test_add_multiple_tracks(self, tracks_list, tracks): - tracks_list.add_tracks(tracks, "run1", select=False) - tracks_list.add_tracks(tracks, "run2", select=False) + def test_add_multiple_tracks(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=False) + tracks_list.add_tracks(motile_run, "run2", select=False) assert tracks_list.tracks_list.count() == 2 - def test_remove_tracks(self, tracks_list, tracks): - tracks_list.add_tracks(tracks, "run1", select=False) + def test_remove_tracks(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=False) item = tracks_list.tracks_list.item(0) tracks_list.remove_tracks(item) assert tracks_list.tracks_list.count() == 0 - def test_selection_changed_emits_signal(self, tracks_list, tracks): + def test_selection_changed_emits_signal(self, tracks_list, motile_run): emitted = [] tracks_list.view_tracks.connect(lambda t, n: emitted.append((t, n))) - tracks_list.add_tracks(tracks, "run1", select=True) + tracks_list.add_tracks(motile_run, "run1", select=True) # Selecting the row triggers _selection_changed assert len(emitted) == 1 assert emitted[0][1] == "run1" + def test_add_solution_tracks_not_wrapped(self, tracks_list, solution_tracks_2d): + """SolutionTracks added to the list should NOT be wrapped in MotileRun.""" + tracks_list.add_tracks(solution_tracks_2d, "imported", select=False) + item = tracks_list.tracks_list.item(0) + widget = tracks_list.tracks_list.itemWidget(item) + assert isinstance(widget.tracks, SolutionTracks) + assert not isinstance(widget.tracks, MotileRun) + + def test_view_tracks_emits_solution_tracks_for_plain_tracks( + self, tracks_list, graph_2d + ): + """The list stores plain Tracks, but view_tracks must emit a + SolutionTracks because the views and actions still need track IDs. + """ + # the fixture graph stores track ids in "track_id", so that has to be + # declared: tracklet_attr is how a caller names an existing column + plain_tracks = Tracks(graph_2d, ndim=3, time_attr="t", tracklet_attr="track_id") + + emitted = [] + tracks_list.view_tracks.connect(lambda t, n: emitted.append((t, n))) + tracks_list.add_tracks(plain_tracks, "plain", select=True) + + # stored as-is, not converted on the way in + item = tracks_list.tracks_list.item(0) + assert tracks_list.tracks_list.itemWidget(item).tracks is plain_tracks + + assert len(emitted) == 1 + converted = emitted[0][0] + assert isinstance(converted, SolutionTracks) + # the conversion must carry over the attributes the views rely on + # rather than re-deriving them + assert converted.scale == plain_tracks.scale + assert converted.ndim == plain_tracks.ndim + assert (converted.segmentation is None) == (plain_tracks.segmentation is None) + # the point of converting: the views need track ids to actually be on + # the graph, not merely named by the FeatureDict + assert converted.features.tracklet_key in converted.graph.node_attr_keys() + assert converted.features.lineage_key in converted.graph.node_attr_keys() + + def test_view_tracks_computes_missing_track_ids(self, tracks_list, graph_2d): + """Tracks with no track id column at all must come out of the + conversion with one computed, not merely declared. + + Tracks imported from a geff that never had track ids land here. On + funtracks < 2.1, passing a FeatureDict to Tracks.__init__ activates the + declared features without computing the missing ones, so the conversion + has to enable them itself. From 2.1 the constructor already computes + them, so this only checks that the outcome is the same either way. + """ + graph_2d.remove_node_attr_key("track_id") + graph_2d.remove_node_attr_key("lineage_id") + plain_tracks = Tracks(graph_2d, ndim=3, time_attr="t") + + emitted = [] + tracks_list.view_tracks.connect(lambda t, n: emitted.append((t, n))) + tracks_list.add_tracks(plain_tracks, "plain", select=True) + + converted = emitted[0][0] + assert converted.features.tracklet_key in converted.graph.node_attr_keys() + assert converted.features.lineage_key in converted.graph.node_attr_keys() + + def test_view_tracks_passes_through_motile_run(self, tracks_list, motile_run): + """A MotileRun is already a SolutionTracks, so it must be emitted + unchanged rather than rebuilt (which would drop its solver params). + """ + emitted = [] + tracks_list.view_tracks.connect(lambda t, n: emitted.append((t, n))) + tracks_list.add_tracks(motile_run, "run1", select=True) + + assert len(emitted) == 1 + assert emitted[0][0] is motile_run + + +# --------------------------------------------------------------------------- +# TracksList — save path fields +# --------------------------------------------------------------------------- + + +class TestTracksListSavePathFields: + def test_save_dir_defaults_to_appdirs(self, tracks_list): + """The save directory starts where the sample data lives, not in the + user's home directory.""" + assert tracks_list.save_dir_line.text() == str(default_save_dir()) + + def test_save_name_empty_before_any_selection(self, tracks_list): + assert tracks_list.save_name_line.text() == "" + + def test_selecting_tracks_fills_save_name(self, tracks_list, motile_run): + """The field holds the bare name; .geff is a fixed label in the UI.""" + tracks_list.add_tracks(motile_run, "run1", select=True) + assert tracks_list.save_name_line.text() == "run1" + + def test_save_name_follows_selection(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.add_tracks(motile_run, "run2", select=True) + assert tracks_list.save_name_line.text() == "run2" + + def test_save_name_strips_geff_suffix_from_tracks_name( + self, tracks_list, motile_run + ): + """Tracks loaded from a geff are named after the store, so the suffix + must not be doubled up.""" + tracks_list.add_tracks(motile_run, "loaded.geff", select=True) + assert tracks_list.save_name_line.text() == "loaded" + assert tracks_list.save_path().name == "loaded.geff" + + def test_user_edit_stops_autofill(self, tracks_list, motile_run): + """Once the user types their own name, selecting another row must not + overwrite it.""" + tracks_list.add_tracks(motile_run, "run1", select=True) + + # textEdited only fires on real user input, so simulate it directly + tracks_list.save_name_line.setText("my_own_name") + tracks_list.save_name_line.textEdited.emit("my_own_name") + + tracks_list.add_tracks(motile_run, "run2", select=True) + + assert tracks_list.save_name_line.text() == "my_own_name" + + def test_save_path_combines_dir_and_name(self, tracks_list, motile_run, tmp_path): + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) + + assert tracks_list.save_path() == tmp_path / "run1.geff" + + def test_save_path_none_when_name_blank(self, tracks_list, tmp_path): + tracks_list.save_dir_line.setText(str(tmp_path)) + tracks_list.save_name_line.setText("") + + assert tracks_list.save_path() is None + + def test_save_path_none_when_dir_blank(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.save_dir_line.setText(" ") + + assert tracks_list.save_path() is None + + def test_save_path_tolerates_typed_suffix(self, tracks_list, tmp_path): + """A user who types the suffix anyway should not get 'x.geff.geff'.""" + tracks_list.save_dir_line.setText(str(tmp_path)) + tracks_list.save_name_line.setText("mine.geff") + + assert tracks_list.save_path() == tmp_path / "mine.geff" + + def test_programmatic_fill_does_not_count_as_user_edit( + self, tracks_list, motile_run + ): + """Auto-filling the field must not mark it as user-edited, or the + first selection would freeze the name forever.""" + tracks_list.add_tracks(motile_run, "run1", select=True) + assert tracks_list._save_name_edited is False + + def test_browse_sets_save_dir(self, tracks_list, tmp_path, monkeypatch): + monkeypatch.setattr( + "motile_tracker.data_views.views_coordinator.tracks_list." + "QFileDialog.getExistingDirectory", + lambda *a, **k: str(tmp_path), + ) + tracks_list._browse_save_dir() + assert tracks_list.save_dir_line.text() == str(tmp_path) + + def test_browse_cancelled_leaves_save_dir(self, tracks_list, monkeypatch): + before = tracks_list.save_dir_line.text() + monkeypatch.setattr( + "motile_tracker.data_views.views_coordinator.tracks_list." + "QFileDialog.getExistingDirectory", + lambda *a, **k: "", + ) + tracks_list._browse_save_dir() + assert tracks_list.save_dir_line.text() == before + # --------------------------------------------------------------------------- # TracksList — save @@ -89,72 +263,191 @@ def test_selection_changed_emits_signal(self, tracks_list, tracks): class TestTracksListSave: - def test_save_tracks_writes_run_dir_when_dialog_accepted( - self, tracks_list, tracks, tmp_path + def test_save_motile_run_writes_geff_at_save_path( + self, tracks_list, motile_run, tmp_path ): - tracks_list.add_tracks(tracks, "run1", select=False) + """The run is written at exactly the path shown in the save fields, + with no intervening subdirectory.""" + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) item = tracks_list.tracks_list.item(0) - tracks_list.save_dialog.exec_ = MagicMock(return_value=True) - tracks_list.save_dialog.selectedFiles = MagicMock(return_value=[str(tmp_path)]) + tracks_list.save_tracks(item) + + save_path = tmp_path / "run1.geff" + assert (save_path / "nodes").exists() + assert list(tmp_path.iterdir()) == [save_path] + + def test_save_emits_tracks_saved_signal(self, tracks_list, motile_run, tmp_path): + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) + item = tracks_list.tracks_list.item(0) + + emitted = [] + tracks_list.tracks_saved.connect(lambda t, p: emitted.append((t, p))) tracks_list.save_tracks(item) - saved_dirs = [p for p in tmp_path.iterdir() if p.is_dir()] - assert len(saved_dirs) == 1 + assert len(emitted) == 1 + assert emitted[0][0] is motile_run + assert emitted[0][1] == tmp_path / "run1.geff" - def test_save_tracks_does_nothing_when_dialog_rejected( - self, tracks_list, tracks, tmp_path + def test_save_motile_run_emits_geff_path_with_params_inside( + self, tracks_list, motile_run, tmp_path ): - tracks_list.add_tracks(tracks, "run1", select=False) + """tracks_saved names the geff store, and the solver params live + inside it rather than beside it. + """ + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) item = tracks_list.tracks_list.item(0) - tracks_list.save_dialog.exec_ = MagicMock(return_value=False) + emitted = [] + tracks_list.tracks_saved.connect(lambda t, p: emitted.append((t, p))) tracks_list.save_tracks(item) + path = emitted[0][1] + assert path.exists() + assert (path / "solver_params.json").exists() + + def test_save_does_nothing_without_a_filename( + self, tracks_list, motile_run, tmp_path + ): + """With no name there is nowhere to save, so warn rather than raise.""" + tracks_list.add_tracks(motile_run, "run1", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) + tracks_list.save_name_line.setText("") + item = tracks_list.tracks_list.item(0) + + emitted = [] + tracks_list.tracks_saved.connect(lambda t, p: emitted.append((t, p))) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tracks_list.save_tracks(item) + + assert len(caught) == 1 assert list(tmp_path.iterdir()) == [] + assert len(emitted) == 0 + + def test_save_creates_missing_save_directory( + self, tracks_list, motile_run, tmp_path + ): + """The default save directory may not exist yet on a fresh install.""" + tracks_list.add_tracks(motile_run, "run1", select=True) + missing = tmp_path / "does" / "not" / "exist" + tracks_list.save_dir_line.setText(str(missing)) + item = tracks_list.tracks_list.item(0) + + tracks_list.save_tracks(item) + + assert (missing / "run1.geff").exists() # --------------------------------------------------------------------------- -# TracksList — imported (non-MotileRun) tracks must be saveable +# TracksList — save SolutionTracks directly (not wrapped in MotileRun) # --------------------------------------------------------------------------- -class TestTracksListSaveImported: - def test_imported_solution_tracks_can_be_saved( +class TestTracksListSaveSolutionTracks: + def test_solution_tracks_saved_directly_to_path( self, tracks_list, solution_tracks_2d, tmp_path ): - """Tracks loaded via ImportDialog (a SolutionTracks, not a MotileRun) - must still be saveable via the save button. + """SolutionTracks are written with write_to_geff at the save path, + not wrapped in a MotileRun.""" + tracks_list.add_tracks(solution_tracks_2d, "imported", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) + item = tracks_list.tracks_list.item(0) - Reproduces the AttributeError seen when saving CSV-imported tracks: - the import path must wrap the SolutionTracks in a MotileRun so - save_tracks → tracks.save(directory) works. - """ - mock_dialog = MagicMock() - mock_dialog.exec_.return_value = QDialog.Accepted - mock_dialog.tracks = solution_tracks_2d - mock_dialog.name = "imported" + tracks_list.save_tracks(item) - with patch( - "motile_tracker.data_views.views_coordinator.tracks_list.ImportDialog", - return_value=mock_dialog, - ): - tracks_list._load_tracks("csv") + assert (tmp_path / "imported.geff").exists() + def test_solution_tracks_save_emits_signal( + self, tracks_list, solution_tracks_2d, tmp_path + ): + tracks_list.add_tracks(solution_tracks_2d, "imported", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) item = tracks_list.tracks_list.item(0) - widget = tracks_list.tracks_list.itemWidget(item) - assert isinstance(widget.tracks, MotileRun) - assert widget.tracks.solver_params is None - tracks_list.save_dialog.exec_ = MagicMock(return_value=True) - tracks_list.save_dialog.selectedFiles = MagicMock(return_value=[str(tmp_path)]) + emitted = [] + tracks_list.tracks_saved.connect(lambda t, p: emitted.append((t, p))) + + tracks_list.save_tracks(item) + + assert len(emitted) == 1 + assert emitted[0][0] is solution_tracks_2d + assert emitted[0][1] == tmp_path / "imported.geff" + + def test_save_respects_edited_name(self, tracks_list, solution_tracks_2d, tmp_path): + """A name the user typed is where the tracks go.""" + tracks_list.add_tracks(solution_tracks_2d, "imported", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) + tracks_list.save_name_line.setText("chosen_by_me") + tracks_list.save_name_line.textEdited.emit("chosen_by_me") + item = tracks_list.tracks_list.item(0) + + tracks_list.save_tracks(item) + + assert (tmp_path / "chosen_by_me.geff").exists() + assert not (tmp_path / "imported.geff").exists() + + +# --------------------------------------------------------------------------- +# TracksList — overwrite confirmation +# --------------------------------------------------------------------------- + + +class TestTracksListOverwrite: + """Saving no longer goes through a file dialog, so this confirmation is the + only thing guarding an existing store.""" + + def _setup(self, tracks_list, tracks, tmp_path): + tracks_list.add_tracks(tracks, "imported", select=True) + tracks_list.save_dir_line.setText(str(tmp_path)) + return tracks_list.tracks_list.item(0) + + def test_no_confirmation_when_path_is_free( + self, tracks_list, solution_tracks_2d, tmp_path, monkeypatch + ): + item = self._setup(tracks_list, solution_tracks_2d, tmp_path) + asked = [] + monkeypatch.setattr( + tracks_list, "_confirm_overwrite", lambda p: asked.append(p) or True + ) + + tracks_list.save_tracks(item) + + assert asked == [] + def test_overwrite_confirmed_writes( + self, tracks_list, solution_tracks_2d, tmp_path, monkeypatch + ): + item = self._setup(tracks_list, solution_tracks_2d, tmp_path) + monkeypatch.setattr(tracks_list, "_confirm_overwrite", lambda p: True) + + tracks_list.save_tracks(item) tracks_list.save_tracks(item) - saved_dirs = [p for p in tmp_path.iterdir() if p.is_dir()] - assert len(saved_dirs) == 1 + assert (tmp_path / "imported.geff").exists() + + def test_overwrite_declined_does_not_write_or_emit( + self, tracks_list, solution_tracks_2d, tmp_path, monkeypatch + ): + item = self._setup(tracks_list, solution_tracks_2d, tmp_path) + tracks_list.save_tracks(item) + + monkeypatch.setattr(tracks_list, "_confirm_overwrite", lambda p: False) + emitted = [] + tracks_list.tracks_saved.connect(lambda t, p: emitted.append((t, p))) + marker = tmp_path / "imported.geff" / "untouched.txt" + marker.write_text("still here") + + tracks_list.save_tracks(item) + + assert marker.read_text() == "still here" + assert len(emitted) == 0 # --------------------------------------------------------------------------- @@ -163,13 +456,30 @@ def test_imported_solution_tracks_can_be_saved( class TestTracksListLoadMotileRun: - def test_load_motile_run_success(self, tracks_list, tracks, tmp_path): - save_dir = tracks.save(tmp_path) + def test_load_motile_run_success(self, tracks_list, motile_run, tmp_path): + save_dir = motile_run.save(tmp_path / "run1.geff") + + tracks_list.file_dialog.exec_ = MagicMock(return_value=True) + tracks_list.file_dialog.selectedFiles = MagicMock(return_value=[str(save_dir)]) + + tracks, name, path = tracks_list.load_motile_run() + + assert isinstance(tracks, MotileRun) + assert name == save_dir.stem + # the run dir is itself the geff store, so loading and saving name the + # same thing + assert path == save_dir + + def test_load_motile_run_adds_to_list_via_load_tracks( + self, tracks_list, motile_run, tmp_path + ): + save_dir = motile_run.save(tmp_path) + tracks_list.dropdown_menu.setCurrentText("Motile Run") tracks_list.file_dialog.exec_ = MagicMock(return_value=True) tracks_list.file_dialog.selectedFiles = MagicMock(return_value=[str(save_dir)]) - tracks_list.load_motile_run() + tracks_list.load_tracks() assert tracks_list.tracks_list.count() == 1 @@ -180,14 +490,75 @@ def test_load_motile_run_bad_path_warns(self, tracks_list, tmp_path): with warnings.catch_warnings(record=True) as caught: warnings.simplefilter("always") - tracks_list.load_motile_run() + result = tracks_list.load_motile_run() assert len(caught) == 1 - assert tracks_list.tracks_list.count() == 0 + assert result is None def test_load_motile_run_dialog_cancelled(self, tracks_list): tracks_list.file_dialog.exec_ = MagicMock(return_value=False) - tracks_list.load_motile_run() + assert tracks_list.load_motile_run() is None + + +# --------------------------------------------------------------------------- +# TracksList — load_internal_tracks +# --------------------------------------------------------------------------- + + +class TestTracksListLoadGeff: + def test_load_internal_tracks_success( + self, tracks_list, solution_tracks_2d, tmp_path + ): + geff_path = tmp_path / "saved_tracks.geff" + write_to_geff(solution_tracks_2d, geff_path) + + tracks_list.dropdown_menu.setCurrentText("Tracks (geff)") + tracks_list.file_dialog.exec_ = MagicMock(return_value=True) + tracks_list.file_dialog.selectedFiles = MagicMock(return_value=[str(geff_path)]) + + tracks_list.load_tracks() + + assert tracks_list.tracks_list.count() == 1 + item = tracks_list.tracks_list.item(0) + widget = tracks_list.tracks_list.itemWidget(item) + assert widget.name.text() == "saved_tracks" + + def test_load_internal_tracks_emits_signal( + self, tracks_list, solution_tracks_2d, tmp_path + ): + geff_path = tmp_path / "saved_tracks.geff" + write_to_geff(solution_tracks_2d, geff_path) + + tracks_list.dropdown_menu.setCurrentText("Tracks (geff)") + tracks_list.file_dialog.exec_ = MagicMock(return_value=True) + tracks_list.file_dialog.selectedFiles = MagicMock(return_value=[str(geff_path)]) + + emitted = [] + tracks_list.tracks_loaded.connect(lambda t, p: emitted.append((t, p))) + + tracks_list.load_tracks() + + assert len(emitted) == 1 + # tracks_loaded hands out the stored object as-is, which is a plain + # Tracks. Only view_tracks converts to SolutionTracks. + assert isinstance(emitted[0][0], Tracks) + assert emitted[0][1] == geff_path + + def test_load_internal_tracks_bad_path_warns(self, tracks_list, tmp_path): + bad_path = tmp_path / "nonexistent.geff" + tracks_list.file_dialog.exec_ = MagicMock(return_value=True) + tracks_list.file_dialog.selectedFiles = MagicMock(return_value=[str(bad_path)]) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + result = tracks_list.load_internal_tracks() + + assert len(caught) == 1 + assert result is None + + def test_load_internal_tracks_dialog_cancelled(self, tracks_list): + tracks_list.file_dialog.exec_ = MagicMock(return_value=False) + assert tracks_list.load_internal_tracks() is None assert tracks_list.tracks_list.count() == 0 @@ -197,21 +568,29 @@ def test_load_motile_run_dialog_cancelled(self, tracks_list): class TestTracksListLoadDispatch: + def test_load_tracks_dispatches_geff_tracks(self, tracks_list): + tracks_list.dropdown_menu.setCurrentText("Tracks (geff)") + with patch.object( + tracks_list, "load_internal_tracks", return_value=None + ) as mock: + tracks_list.load_tracks() + mock.assert_called_once() + def test_load_tracks_dispatches_motile_run(self, tracks_list): tracks_list.dropdown_menu.setCurrentText("Motile Run") - with patch.object(tracks_list, "load_motile_run") as mock: + with patch.object(tracks_list, "load_motile_run", return_value=None) as mock: tracks_list.load_tracks() mock.assert_called_once() def test_load_tracks_dispatches_csv(self, tracks_list): tracks_list.dropdown_menu.setCurrentText("External tracks from CSV") - with patch.object(tracks_list, "_load_tracks") as mock: + with patch.object(tracks_list, "_load_tracks", return_value=None) as mock: tracks_list.load_tracks() mock.assert_called_once_with(import_type="csv") def test_load_tracks_dispatches_geff(self, tracks_list): tracks_list.dropdown_menu.setCurrentText("External tracks from geff") - with patch.object(tracks_list, "_load_tracks") as mock: + with patch.object(tracks_list, "_load_tracks", return_value=None) as mock: tracks_list.load_tracks() mock.assert_called_once_with("geff") @@ -222,17 +601,19 @@ def test_load_tracks_dispatches_geff(self, tracks_list): class TestTracksListLoadExternal: - def test_load_tracks_accepted_adds_tracks(self, tracks_list, tracks): + def test_load_tracks_accepted_adds_tracks(self, tracks_list, motile_run, tmp_path): mock_dialog = MagicMock() mock_dialog.exec_.return_value = QDialog.Accepted - mock_dialog.tracks = tracks + mock_dialog.tracks = motile_run mock_dialog.name = "imported" + mock_dialog.source_path = tmp_path / "test.csv" + tracks_list.dropdown_menu.setCurrentText("External tracks from CSV") with patch( "motile_tracker.data_views.views_coordinator.tracks_list.ImportDialog", return_value=mock_dialog, ): - tracks_list._load_tracks("csv") + tracks_list.load_tracks() assert tracks_list.tracks_list.count() == 1 @@ -244,7 +625,7 @@ def test_load_tracks_rejected_adds_nothing(self, tracks_list): "motile_tracker.data_views.views_coordinator.tracks_list.ImportDialog", return_value=mock_dialog, ): - tracks_list._load_tracks("csv") + assert tracks_list._load_tracks("csv") is None assert tracks_list.tracks_list.count() == 0 @@ -257,7 +638,7 @@ def test_load_tracks_accepted_but_none_tracks(self, tracks_list): "motile_tracker.data_views.views_coordinator.tracks_list.ImportDialog", return_value=mock_dialog, ): - tracks_list._load_tracks("csv") + assert tracks_list._load_tracks("csv") is None assert tracks_list.tracks_list.count() == 0 @@ -268,8 +649,8 @@ def test_load_tracks_accepted_but_none_tracks(self, tracks_list): class TestTracksListExport: - def test_show_export_dialog_called(self, tracks_list, tracks): - tracks_list.add_tracks(tracks, "run1", select=False) + def test_show_export_dialog_called(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=False) item = tracks_list.tracks_list.item(0) with patch( @@ -278,8 +659,8 @@ def test_show_export_dialog_called(self, tracks_list, tracks): tracks_list.show_export_dialog(item) mock_export.assert_called_once() - def test_show_export_dialog_emits_request_colormap(self, tracks_list, tracks): - tracks_list.add_tracks(tracks, "run1", select=False) + def test_show_export_dialog_emits_request_colormap(self, tracks_list, motile_run): + tracks_list.add_tracks(motile_run, "run1", select=False) item = tracks_list.tracks_list.item(0) emitted = [] diff --git a/tests/data_views/views_coordinator/test_tracks_viewer.py b/tests/data_views/views_coordinator/test_tracks_viewer.py index c41a4ab1..8c38b0af 100644 --- a/tests/data_views/views_coordinator/test_tracks_viewer.py +++ b/tests/data_views/views_coordinator/test_tracks_viewer.py @@ -417,6 +417,59 @@ def test_get_instance_returns_new_instance_for_different_viewer(self, viewer, qa TracksViewer._instance = tv1 +class TestTracksSignalCleanup: + """Tests that a superseded TracksViewer unsubscribes from its tracks object.""" + + def test_superseded_instance_disconnects_from_tracks( + self, viewer, solution_tracks_2d, qapp + ): + """Showing one tracks object in successive viewers must not stack up + listeners on that object's refresh signal. + + Regression: get_instance() replaced _instance without telling the outgoing + TracksViewer to disconnect, and update_tracks only ever disconnects the + instance's *own* previous tracks (None for a fresh instance). So every new + viewer added another _refresh listener to the same Tracks object, and each + subsequent edit refreshed every viewer ever built - editing cost grew + linearly with the number of viewers (~2x in a 3-round benchmark). + """ + tv1 = TracksViewer.get_instance(viewer) + tv1.update_tracks(tracks=solution_tracks_2d, name="test") + assert len(solution_tracks_2d.refresh) == 1 + + v2 = napari.Viewer(show=False) + try: + tv2 = TracksViewer.get_instance(v2) + tv2.update_tracks(tracks=solution_tracks_2d, name="test") + + assert len(solution_tracks_2d.refresh) == 1, ( + "the superseded TracksViewer is still connected to " + "tracks.refresh; listeners accumulate per viewer" + ) + assert len(solution_tracks_2d.action_applied) == 1, ( + "the superseded TracksViewer is still connected to " + "tracks.action_applied" + ) + finally: + v2.close() + qapp.processEvents() + TracksViewer._instance = tv1 + + def test_disconnects_when_viewer_window_destroyed(self, solution_tracks_2d, qapp): + """Closing a viewer must unsubscribe its TracksViewer from the tracks.""" + v = napari.Viewer(show=False) + tv = TracksViewer.get_instance(v) + tv.update_tracks(tracks=solution_tracks_2d, name="test") + assert len(solution_tracks_2d.refresh) == 1 + + v.close() + qapp.processEvents() + + assert len(solution_tracks_2d.refresh) == 0, ( + "tracks.refresh still holds a listener from the closed viewer" + ) + + class TestUndoRedo: """Tests for undo/redo functionality.""" diff --git a/tests/import_export/test_import_dialog.py b/tests/import_export/test_import_dialog.py index 929ab8fe..fc159766 100644 --- a/tests/import_export/test_import_dialog.py +++ b/tests/import_export/test_import_dialog.py @@ -463,6 +463,44 @@ def test_geff_import_with_segmentation( assert dialog.tracks.graph.nodes[node_id]["area"] > 0 +def test_geff_import_source_path_is_geff_group_not_container( + qtbot, tmp_path, graph_2d, monkeypatch +): + """source_path must name the geff group that was read, not the zarr + container it was found inside. + + export_to_geff writes a container with the graph in a nested `tracks.geff` + group. Listeners on TracksList.tracks_loaded use this path to find data + stored alongside the tracks, so pointing at the container would be + ambiguous when it holds more than one group. + """ + monkeypatch.setattr(ImportDialog, "_resize_dialog", lambda self: None) + + tracks = Tracks(graph_2d, ndim=3, time_attr="t", tracklet_attr="track_id") + container = tmp_path / "container.zarr" + export_to_geff(tracks, container) + + dialog = ImportDialog(import_type="geff") + qtbot.addWidget(dialog) + dialog.import_widget._load_geff(container) + assert dialog.import_widget.root is not None + + seg_combo = dialog.prop_map_widget.mapping_widgets["seg_id"] + seg_combo.setCurrentText("None") + dialog.prop_map_widget._update_props_left() + + dialog._finish() + + assert dialog.tracks is not None + assert dialog.source_path is not None + # the geff group lives inside the container, not at its root + assert dialog.source_path != container + assert container in dialog.source_path.parents + assert (dialog.source_path / ".zattrs").exists() or ( + dialog.source_path / "zarr.json" + ).exists() + + def test_geff_import_without_area_computes_area( qtbot, tmp_path, graph_2d_without_segmentation, segmentation_2d, monkeypatch ): @@ -845,9 +883,10 @@ def test_motile_run_save_load(tmp_path, graph_2d): ndim=3, time_attr="t", ) - run_dir = run.save(tmp_path) + run_dir = run.save(tmp_path / "test_run.geff") - assert (run_dir / "tracks.geff").exists() + # the run dir is itself the geff store, with the run's own files inside it + assert (run_dir / "nodes").exists() assert (run_dir / "solver_params.json").exists() assert (run_dir / "attrs.json").exists() @@ -867,10 +906,15 @@ def test_motile_run_load_backward_compat(tmp_path, graph_2d): ndim=3, time_attr="t", ) - run_dir = run.save(tmp_path) - - # Simulate old save format: rename tracks.geff → tracks - (run_dir / "tracks.geff").rename(run_dir / "tracks") + saved = run.save(tmp_path / "old_run.geff") + + # Simulate the old save format: the graph in a 'tracks' subdirectory of a + # run directory, rather than the run directory being the geff store itself + run_dir = tmp_path / "run" + run_dir.mkdir() + saved.rename(run_dir / "tracks") + for name in ("solver_params.json", "attrs.json"): + (run_dir / "tracks" / name).rename(run_dir / name) assert not (run_dir / "tracks.geff").exists() loaded = MotileRun.load(run_dir) diff --git a/tests/motile/backend/test_motile_run.py b/tests/motile/backend/test_motile_run.py index 7c4dbe90..4c25aa23 100644 --- a/tests/motile/backend/test_motile_run.py +++ b/tests/motile/backend/test_motile_run.py @@ -1,8 +1,131 @@ +import warnings + import numpy as np from motile_tracker.motile.backend import MotileRun, SolverParams +def test_geff_path_finds_saved_geff(tmp_path, graph_2d): + """A run saved by the current version is itself the geff store.""" + run = MotileRun(graph=graph_2d, run_name="test", solver_params=SolverParams()) + run_dir = run.save(tmp_path / "my_run.geff") + + geff = MotileRun.geff_path(run_dir) + assert geff == run_dir + assert geff.exists() + + +def test_geff_path_finds_nested_tracks_geff(tmp_path): + """Runs saved by the previous version nested the graph in tracks.geff.""" + run_dir = tmp_path / "run" + (run_dir / "tracks.geff").mkdir(parents=True) + + assert MotileRun.geff_path(run_dir) == run_dir / "tracks.geff" + + +def test_save_writes_params_inside_the_geff(tmp_path, graph_2d): + """Solver params live inside the store, not beside it.""" + run = MotileRun(graph=graph_2d, run_name="test", solver_params=SolverParams()) + run_dir = run.save(tmp_path / "my_run.geff") + + assert (run_dir / "solver_params.json").exists() + assert (run_dir / "attrs.json").exists() + assert (run_dir / "nodes").exists() + + +def test_resave_is_quiet(tmp_path, graph_2d): + """Overwriting a run must not warn about its own files. + + The run keeps solver params, attrs, gaps and input points inside the geff + store. Zarr walks the directory while the geff is being replaced and warns + once per file it does not recognise, which is expected and not actionable. + """ + run = MotileRun(graph=graph_2d, run_name="test", solver_params=SolverParams()) + path = tmp_path / "my_run.geff" + run.save(path) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + run.save(path) + + unrecognized = [ + w for w in caught if "not recognized as a component" in str(w.message) + ] + non_geff = [w for w in caught if "non-geff members" in str(w.message)] + assert unrecognized == [] + assert non_geff == [] + + +def test_resave_preserves_params(tmp_path, graph_2d): + """Writing a geff only replaces geff-controlled groups, so the run's own + files survive being saved over.""" + run = MotileRun(graph=graph_2d, run_name="test", solver_params=SolverParams()) + path = tmp_path / "my_run.geff" + run.save(path) + run.save(path) + + assert (path / "solver_params.json").exists() + assert MotileRun.load(path).solver_params == run.solver_params + + +def test_geff_path_falls_back_to_tracks_dir(tmp_path): + """Intermediate-format runs stored the graph in a 'tracks' zarr.""" + run_dir = tmp_path / "run" + run_dir.mkdir() + (run_dir / "tracks").mkdir() + + assert MotileRun.geff_path(run_dir) == run_dir / "tracks" + + +def test_geff_path_none_for_v1_run(tmp_path): + """v1 runs stored the graph as graph.json, so there is no geff to report.""" + run_dir = tmp_path / "run" + run_dir.mkdir() + (run_dir / "graph.json").write_text("{}") + + assert MotileRun.geff_path(run_dir) is None + + +def test_load_run_dir_renamed_to_non_timestamp(tmp_path, graph_2d): + """A run directory the user renamed must still load. + + The name and time come from the attrs file, so they survive a rename that + _unpack_id could not parse. + """ + run = MotileRun(graph=graph_2d, run_name="my_run", solver_params=SolverParams()) + run_dir = run.save(tmp_path / "my_run.geff") + renamed = run_dir.rename(tmp_path / "not_a_timestamp") + + loaded = MotileRun.load(renamed) + + assert loaded.run_name == "my_run" + assert loaded.time == run.time + + +def test_load_falls_back_to_unpack_id_without_attrs(tmp_path, graph_2d): + """Runs saved before the name/time were written to attrs still load by + unpacking the timestamped directory name.""" + run = MotileRun(graph=graph_2d, run_name="test", solver_params=SolverParams()) + # reproduce the old layout: a directory named by _make_id + run_dir = run.save(tmp_path / run._make_id()) + (run_dir / "attrs.json").unlink() + + loaded = MotileRun.load(run_dir) + + assert loaded.run_name == "test" + # the directory-name timestamp only has second granularity + assert loaded.time == run.time.replace(microsecond=0) + + +def test_resolve_name_and_time_falls_back_to_dir_stem(tmp_path): + """With neither attrs nor a parseable directory name, the directory name + is used and the time is left for __init__ to fill in.""" + time, name = MotileRun._resolve_name_and_time(tmp_path / "some_run", None) + + assert name == "some_run" + assert time is None + + def test_save_load(tmp_path, graph_2d): run_name = "test" scale = [1.0, 2.0, 3.0] @@ -12,7 +135,7 @@ def test_save_load(tmp_path, graph_2d): solver_params=SolverParams(), scale=scale, ) - path = run.save(tmp_path) + path = run.save(tmp_path / "test.geff") newrun = MotileRun.load(path) assert set(run.graph.node_ids()) == set(newrun.graph.node_ids()) assert {tuple(e) for e in run.graph.edge_list()} == { @@ -20,7 +143,9 @@ def test_save_load(tmp_path, graph_2d): } assert run.run_name == newrun.run_name assert np.array_equal(np.asarray(run.segmentation), np.asarray(newrun.segmentation)) - assert run.time.replace(microsecond=0) == newrun.time + # the time now round-trips exactly: it comes from the attrs file rather + # than from the second-granularity timestamp in the directory name + assert run.time == newrun.time assert run.gaps == newrun.gaps assert run.scale == newrun.scale assert run.solver_params == newrun.solver_params