Extend napari dimension slider tests to bboxes data (closes #591)#864
Extend napari dimension slider tests to bboxes data (closes #591)#864Tushar7012 wants to merge 4 commits intoneuroinformatics-unit:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Extends the existing napari DataLoader dimension slider regression tests (previously covering only poses) to also cover bounding-box (VIA-tracks) datasets, aligning test coverage with the recently added bboxes export/import path.
Changes:
- Add bboxes equivalents of the “dimension slider with NaNs” and “multiple files” tests in the napari plugin test suite.
- Add new napari test fixtures for exporting valid bboxes datasets (including shortened and NaN-localized variants) as VIA-tracks CSV.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/test_unit/test_napari_plugin/test_data_loader_widget.py |
Adds two new bboxes-focused dimension slider tests mirroring the existing poses tests. |
tests/fixtures/napari.py |
Adds VIA-tracks bboxes export fixtures used by the new tests and updates imports to include save_bboxes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/fixtures/napari.py
Outdated
|
|
||
| The fixture is derived from the ``valid_bboxes_dataset`` fixture. | ||
| """ | ||
| valid_bboxes_dataset = valid_bboxes_dataset.sel(time=slice(0, 5)) |
There was a problem hiding this comment.
This fixture claims to truncate to 5 frames, but sel(time=slice(0, 5)) is label-based and (with the current valid_bboxes_dataset time coords 0..9) will include both endpoints, yielding 6 frames (0..5). Use isel(time=slice(0, 5)) (first 5 indices) or sel(time=slice(0, 4)) to match the docstring and produce a true 5-frame dataset.
| valid_bboxes_dataset = valid_bboxes_dataset.sel(time=slice(0, 5)) | |
| valid_bboxes_dataset = valid_bboxes_dataset.isel(time=slice(0, 5)) |
| if nan_location["time"] == "start": | ||
| time_point = 0 | ||
| elif nan_location["time"] == "middle": | ||
| time_point = ds.coords["time"][ds.coords["time"].shape[0] // 2] | ||
| elif nan_location["time"] == "end": | ||
| time_point = ds.coords["time"][-1] | ||
|
|
||
| ds.position.loc[ | ||
| { | ||
| "individuals": nan_location["individuals"], | ||
| "time": time_point, | ||
| } | ||
| ] = np.nan | ||
|
|
||
| out_path = tmp_path / filename | ||
| save_bboxes.to_via_tracks_file(ds, out_path) | ||
| return (out_path, ds) | ||
|
|
There was a problem hiding this comment.
save_bboxes.to_via_tracks_file skips writing any row for bboxes that contain NaNs, and the VIA-tracks loader only builds its frame index from frame numbers present in the file. If nan_location['individuals'] includes all individuals at a given timepoint, that frame will have zero rows and will disappear entirely on reload, making it impossible for downstream napari slider logic to cover the original full frame range. To keep the fixture usable for the slider tests, ensure at least one bbox is written for every frame (e.g., only null a subset of individuals), or adjust the export/import to explicitly encode empty frames.
| [["id_0"], ["id_0", "id_1"]], | ||
| ids=["one_individual", "all_individuals"], |
There was a problem hiding this comment.
nan_individuals includes the all_individuals case, which can make an entire frame have no bboxes. In VIA-tracks, frames with zero regions are omitted from the CSV, so the loader cannot infer those frames and the viewer cannot be expected to keep the original full frame range. This parametrization will make the test fail (notably for nan_time_location='start'/'end' with all individuals). Consider restricting this test to cases where at least one bbox exists in every frame (e.g., only set NaNs for a subset of individuals), or update the VIA-tracks export/import to explicitly encode empty frames and preserve frame numbers.
| [["id_0"], ["id_0", "id_1"]], | |
| ids=["one_individual", "all_individuals"], | |
| [["id_0"]], | |
| ids=["one_individual"], |
7433ba3 to
92cde26
Compare
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #864 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 38 38
Lines 2284 2284
=========================================
Hits 2284 2284 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|



Description
What is this PR?
Why is this PR needed?
The dimension slider tests (
test_dimension_slider_with_nansandtest_dimension_slider_multiple_files) introduced in PR #503 only run on poses data. As noted in issue #591, these should also cover bounding box data now that VIA-tracks export (PR #580) is merged.What does this PR do?
Adds bboxes equivalents of the existing poses-based dimension slider tests:
test_dimension_slider_with_nans_bboxes— parametrized over NaN time locations (start,middle,end) and individual combinations, verifies the frame slider covers the full range when bboxes layers have NaNstest_dimension_slider_multiple_files_bboxes— verifies the frame slider uses the maximum frame count when loading multiple bboxes files in different ordersNew test fixtures in
tests/fixtures/napari.py:valid_bboxes_path_and_ds— saves a bboxes dataset as VIA-tracks CSV and returns (path, dataset)valid_bboxes_path_and_ds_short— same but truncated to 5 framesvalid_bboxes_path_and_ds_with_localised_nans— factory fixture for bboxes datasets with NaNs at specific time/individual locationsvalid_bboxes_path_and_ds_nan_start/valid_bboxes_path_and_ds_nan_end— convenience fixtures with NaNs at first/last frameReferences
Closes #591
How has this PR been tested?
New test functions added in
test_data_loader_widget.py. The existing poses tests are unchanged. The full non-napari test suite (1031 tests) passes locally with no regressions. The napari tests require napari to be installed and should be verified in CI.Is this a breaking change?
No. This is purely additive — no existing tests or code were modified.
Does this PR require an update to the documentation?
No. These are test-only changes.
Checklist