-
Notifications
You must be signed in to change notification settings - Fork 2
Fix/channel persistence on dataset switch #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
hongquanli
wants to merge
26
commits into
maragall:main
Choose a base branch
from
Cephla-Lab:fix/channel-persistence-on-dataset-switch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Fix/channel persistence on dataset switch #2
hongquanli
wants to merge
26
commits into
maragall:main
from
Cephla-Lab:fix/channel-persistence-on-dataset-switch
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Feature/live refresh + sim
Hide status label
- Add logging for exception handlers instead of silent pass - Extract duplicate dark theme code to shared _apply_dark_theme() - Add type hints for class attributes in LightweightViewer/MainWindow - Add closeEvent handlers to properly clean up TiffFile handles - Extract constants: TIFF_EXTENSIONS, LIVE_REFRESH_INTERVAL_MS - Remove unused imports (QPushButton) and parameters (index) - Fix incomplete docstring placeholder 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Improve code quality and resource management
- Add latest_file_count to signature to detect when files are written - Only include timepoints in t_set when they have actual files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* Fix visible_axes to use numeric indices
Use (-2, -1) instead of ("y", "x") to avoid NDV misinterpreting
named axes and attempting 3D volume rendering.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Disable 3D mode to prevent OpenGL texture issues
Add viewer_options={"show_3d_button": False} to prevent ndv from
attempting 3D volume rendering which can cause OpenGL errors with
large images when channel_axis conflicts with visible_axes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Fix 3D volume rendering by renaming z_level to z
NDV's guess_z_axis() only recognizes "z", "depth", "focus" as z-axis
names. Renamed "z_level" to "z" so NDV correctly detects it, preventing
the channel axis from being incorrectly used as the z-axis in 3D mode.
Also reverts the 3D button disabling to allow 3D volume viewing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Disable 3D mode for images exceeding GPU texture limits
3D volume rendering requires all dimensions to fit within
GL_MAX_3D_TEXTURE_SIZE (typically 2048). Hide 3D button when
image dimensions exceed this limit to prevent OpenGL errors.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add automatic downsampling for 3D volume rendering
Images exceeding GPU texture limits (2048px) are now automatically
downsampled using xarray's coarsen() with dask lazy evaluation.
This enables 3D volume viewing for large microscopy images.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add --n-z option to simulator for multiple z-slices
3D volume rendering requires multiple z-slices. Changed simulator
to support --n-z parameter (default: 1) for generating 3D test data.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Disable 3D button for images exceeding GPU texture limits
ndv's lazy data loading bypasses downsampling when fetching 3D data,
so images >2048px cannot be rendered in 3D without crashing. Hide the
3D button for large images to prevent OpenGL errors.
3D rendering still works for images ≤2048px.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add automatic 3D downsampling for large volumes
- Add Downsampling3DXarrayWrapper that intercepts NDV data requests
and automatically downsamples 3D volumes exceeding OpenGL texture
limits (2048px)
- Use dimension names (z, y, x) to identify spatial dimensions,
preserving channel dimensions during downsampling
- Use nearest neighbor interpolation (order=0) for 90x faster
downsampling (~0.2s vs ~18s for 4168x4168 volumes)
- Rename z_level dimension to z for proper NDV recognition
- Add comprehensive test suite for downsampling wrapper
This enables 3D volume rendering for images of any size by
transparently downsampling when switching to 3D view mode.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Query GL_MAX_3D_TEXTURE_SIZE dynamically from GPU
- Add lazy query for GL_MAX_3D_TEXTURE_SIZE when OpenGL context exists
- Remove redundant allow_3d logic (downsampling wrapper handles large volumes)
- Remove unused import in test file
- Fall back to conservative 2048 when no GL context available
Addresses PR review comments.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Address PR review comments (round 2)
- Remove redundant LAZY_LOADING_AVAILABLE check in supports() method
- Add minimal error handling around ndimage_zoom with fallback
- Add backward compatibility for --z CLI argument (alias for --n-z)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Address PR review comments (round 3)
- Fix zoom_factors dimension mismatch: skip dropped dimensions (integer indices)
- Update comment to match actual label format (F=/C= not FOV=/CH=)
- Use getattr for defensive access to vispy's private _current_canvas
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Use consistent type annotation for xr.DataArray
Remove string quotes from type annotation since xr is available
in the LAZY_LOADING_AVAILABLE block.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Address minor PR review comments
- Use general "much faster" instead of specific "90x faster" claim
- Use 65536 in randint to include full uint16 range
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Scale z independently while preserving xy aspect ratio
- z: only downsampled if z itself exceeds GL texture limit
- x/y: use same scale factor based on max(x,y) to preserve aspect ratio
- Updated test to verify new behavior
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Code review fixes: remove dead code, update docstring, add test
- Remove unused spatial_names variable
- Update isel() docstring to describe z-independent scaling strategy
- Add test for large z (> limit) being downsampled independently
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Address PR review comments (round 4)
- Improve GL context detection using vispy backend check
- Add test for integer indexing (dimension dropping)
- Fix inconsistent randint bounds (65535 → 65536)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Use tolerance-based assertions for zoom rounding
scipy.ndimage.zoom may produce off-by-one results due to rounding.
Changed exact equality assertions to allow ±1-2 pixel tolerance.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Set up continuous integration with: - Black formatting check - Pytest with xvfb for Qt headless testing - Conda environment from environment.yml 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
scipy is required for lazy loading (LAZY_LOADING_AVAILABLE flag). Without it, the downsampling wrapper tests are skipped in CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* Display channel names instead of numeric indices - Extract channel names from OME metadata or filenames - Store channel names in xarray attrs for later use - Add _update_channel_labels() method to update NDV's internal _lut_controllers with actual channel names - Use QTimer delay to ensure NDV initialization completes before updating labels 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments for channel name display - Keep xarray coords numeric instead of string channel names to avoid luts/coords type mismatch (addresses comments #2 and #3) - Replace fixed 500ms QTimer delay with retry mechanism that polls for _lut_controllers readiness with bounded retries (addresses comment #4) - Add documentation noting _lut_controllers is a private API that may change in future ndv versions (addresses comment #1) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 2) - Fix single-TIFF path to use numeric coords instead of string channel names, consistent with OME-TIFF path (comment #1) - Add generation counter to cancel stale retry callbacks when viewer is replaced by a new dataset load (comment #2) - Add hasattr check for synchronize() method with debug logging when missing (comment #3) - Remove placeholder issue URL, keep explanation about private API (comment #4) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add comment explaining synchronize() purpose 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 4) - Fix misleading "all axes" comments to accurately describe which axes use numeric indices vs actual values (comments #3, #5) - Preserve partial channel names from OME metadata instead of discarding all when count doesn't match n_c (comment #4) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 5) - Initialize _channel_label_generation in __init__ for clarity instead of relying on getattr defaults (comment #5) - Add debug logging when channel label update succeeds (comment #6) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 6) - Rename 'channels' to 'channel_names' in single-TIFF path for consistency with OME-TIFF path (comment #1) - Initialize _pending_channel_label_retries in __init__ alongside _channel_label_generation for consistency (comment #3) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add unit tests for channel name display feature Test coverage includes: - OME-TIFF channel name extraction and fallback logic - Single-TIFF channel name extraction from filenames - Retry mechanism with generation counter - Channel label update on NDV controllers - Integration tests for end-to-end behavior 25 tests, all passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 7) - Clarify that numeric coordinates convention applies to both OME-TIFF and single-TIFF paths (comment #5) - Document graceful failure mode if private _lut_controllers API changes (comment #9) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 8) - Change timeout log from debug to warning for user visibility (comment #2) - Add pytest install note to tests/README.md (comment #3) - Remove unused pytest import (comment #8) - Remove unused ch2 variable (comment #6) - Remove unnecessary pending_retries assignment (comment #7) - Add explanatory comments to except clauses (comments #9, #10, #11) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments (round 9) - Fix debug log to track actual updated names instead of slicing list (handles controller gaps correctly) (comment #15) - Remove unnecessary getattr for _pending_channel_label_retries since it's now initialized in __init__ (comment #21) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix channel labels not updating during in-place refresh During live acquisition, if _try_inplace_ndv_update succeeds, the code returned early without scheduling channel label updates. This caused channel labels to show numeric indices instead of names (e.g., "DAPI", "GFP") after in-place data swaps. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix CI: black formatting and skip downsampling tests when unavailable - Apply black formatting to ndviewer_light.py and test_channel_names.py - Add pytest skipif marker to test_downsampling_wrapper.py to skip tests when Downsampling3DXarrayWrapper is not available (requires scipy) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add scipy to CI environment for 3D downsampling tests scipy.ndimage.zoom is used by Downsampling3DXarrayWrapper for resampling large 3D volumes to fit within OpenGL texture limits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove accidentally committed IDE config files - Remove .claude/agents/ directory (IDE-specific configuration) - Remove .coverage file (test artifact) - Add both to .gitignore to prevent future commits 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Replace line number references with method names in tests Line numbers drift as code evolves, making references misleading. Method names are more stable and searchable with IDE navigation. Updated: - test_channel_names.py: All inline comments now reference method names - tests/README.md: Documentation uses method names instead of line numbers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix README: "three main areas" → "five main areas" The test file has five test classes, not three. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Change synchronize() fallback log from debug to warning If synchronize() method doesn't exist on LUT controller, channel labels may not appear in the UI. A warning is more appropriate than debug level. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Use _ = for intentionally unused ET.SubElement return value Makes explicit that the return value is intentionally unused. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Extract constants and helper for channel label update retry - Add CHANNEL_LABEL_UPDATE_MAX_RETRIES (20) constant - Add CHANNEL_LABEL_UPDATE_RETRY_DELAY_MS (100) constant - Extract _initiate_channel_label_update() helper method to DRY up duplicated code between _store_data_from_loader and _set_ndv_data 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Enables editable install via `pip install -e .` for use as a dependency in other projects. Pins ndv to >=0.4.0,<0.5.0 due to private API usage. Updates README with library installation and usage instructions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* Add acquisition metadata extraction for XY and Z scale Extract physical pixel sizes from acquisition metadata to ensure correct XY and Z scaling in the viewer: - For OME-TIFF: Parse PhysicalSizeX/Y/Z from OME-XML metadata - Supports multiple OME namespace versions (2016-06, 2015-01, 2013-06) - Handles unit conversion (nm, mm, m to micrometers) - For single-TIFF: Read from acquisition_parameters.json - Supports common key names (pixel_size_um, dz_um, z_step, etc.) Scale metadata is stored in xarray attrs as: - pixel_size_um: XY pixel size in micrometers - dz_um: Z step size in micrometers - pixel_size_x_um, pixel_size_y_um, pixel_size_z_um: Individual axis sizes When scale metadata is found, it's logged at data load time. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Preserve physical aspect ratio in 3D volume downsampling When physical pixel sizes (pixel_size_um, dz_um) are available in xarray attrs, the downsampling now maintains correct physical aspect ratio: - Compute physical dimensions using pixel sizes - Scale uniformly in physical space to fit texture limits - Z dimension is resampled to match physical proportions - Falls back to simple independent scaling when pixel sizes unknown This ensures the 3D volume rendering displays correct proportions even when XY is downsampled but Z would not need it (or vice versa). Added 3 new tests for aspect ratio preservation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add TIFF metadata tag reading for pixel size fallback For single-TIFF datasets, pixel size can now be read from: 1. acquisition_parameters.json (primary source) 2. TIFF ImageDescription tag (JSON metadata from microscopy software) 3. TIFF XResolution/YResolution tags with inch or cm units This provides automatic pixel size detection when TIFFs contain proper resolution metadata, without requiring a separate JSON file. Added 5 new tests for TIFF tag reading. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Simplify downsampling: store scale metadata without Z resampling Remove Z upsampling for aspect ratio correction - it was wasteful and introduced interpolation artifacts. NDV/vispy Volume assumes isotropic voxels; proper aspect ratio support would require deeper renderer integration. Now: - XY scaled uniformly (preserves XY aspect ratio) - Z scaled independently only if exceeds texture limit - Physical pixel sizes stored in attrs for reference (pixel_size_um, dz_um) The scale metadata can be used for measurements, exports, or future NDV versions that support anisotropic voxels via scale transforms. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Apply voxel scale transform for correct 3D aspect ratio Instead of wastefully upsampling Z data, apply a scale transform to the vispy Volume visual to display anisotropic voxels with correct proportions. How it works: 1. Physical pixel sizes (pixel_size_um, dz_um) are read from metadata 2. Z scale factor is computed as dz / pixel_size_xy 3. Monkey-patch VispyArrayCanvas.add_volume to apply STTransform 4. Volume visual is scaled by (1, 1, z_scale) for correct aspect ratio Example: if pixel_size_xy = 0.325 µm and dz = 1.5 µm: - z_scale = 1.5 / 0.325 = 4.615 - Z dimension appears 4.615x larger in 3D view (correct physical proportion) This approach: - No data interpolation or upsampling - No additional memory usage - Correct visual proportions in 3D rendering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add tube lens correction and improve metadata extraction - Compute pixel size from sensor_pixel_size_um and magnification with tube lens ratio correction (actual_mag = nominal_mag × tube_lens/obj_tube_lens) - Support 'acquisition parameters.json' filename variant (with space) - Support 'dz(um)' key format for Z step size - Add manual 3D visualization test with synthetic volume data - Add tests for computed pixel size and tube lens correction 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix CI: apply black formatting and make TIFF tests more robust - Apply black formatting to ndviewer_light.py - Make TIFF pixel size tests skip gracefully when resolution tag handling varies between tifffile versions or environments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix downsampling wrapper tests skip condition Check both NDV_AVAILABLE and LAZY_LOADING_AVAILABLE flags since the Downsampling3DXarrayWrapper is only defined when both dependencies are available. This fixes test failures in CI where scipy (required for lazy loading) may not be installed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments - Fix thread safety: store voxel scale as instance attribute instead of global variable to prevent race conditions with multiple volumes - Add validation for negative/zero pixel sizes in TIFF metadata parsing - Document sanity check range (0.01-100 µm) for pixel sizes - Add explanatory comment for JSON parsing except clause - Simplify always-true condition in TIFF resolution tag parsing - Fix misleading docstring about isotropic voxels in test file 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address second round of PR review comments - Remove duplicate VolumeVisual.__init__ assignment (was on lines 92 and 137) - Fix docstring order in read_tiff_pixel_size to match code priority (ImageDescription checked first, then XResolution/YResolution) - Simplify confusing double negation in test skip conditions by using pytest.importorskip() directly instead of skipif(not importorskip(...)) - Rename tmpdir to tmp_dir for PEP 8 snake_case compliance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address third round of PR review comments - Add validation for dz parameter (must be strictly positive) - Add validation for computed pixel size from magnification (0.01-100 µm) - Add positive value validation in to_micrometers for OME metadata - Update read_tiff_pixel_size docstring to explain ResolutionUnit handling (unit_value=1 rejected because it cannot be reliably converted) - Fix test_tiff_without_metadata to use pytest.importorskip - Add temp directory cleanup note in test_3d_visualization.py docstring - Remove unrelated .claude/agents/ config files from PR 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address fourth round of PR review comments - Add validation for direct pixel_size from JSON (0.01-100 µm range) - Remove duplicate pytestmark assignment in test_downsampling_wrapper.py (kept the WRAPPER_AVAILABLE check which covers all dependencies) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Remove test skip - dependencies should be installed in CI Tests should run (not skip) since scipy and ndv are in environment.yml. If dependencies are missing, tests should fail to catch CI misconfiguration. Also removed duplicate scipy entry from environment.yml. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address fifth round of PR review comments - Add dimension check (<3D data) and try-except in add_volume camera adjustment - Call original _create_vertex_data when voxel scale is None - Remove redundant ".//Pixels" XML fallback (.//{*} already matches any namespace) - Fix tube structure to use physical coordinates (consistent with spheres/helix) - Update temp directory cleanup documentation with retention policy - Use <= for upper bound in sanity checks (include 100 µm edge case) - Add test for negative/zero OME physical size values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
When switching from an acquisition with multiple channels (e.g., 405, 488, 561) to one with fewer channels (e.g., 405 only), the old channel controls would persist in the viewer UI, showing phantom channels that don't exist in the new dataset. Changes: - Add _data_structure_changed() to detect when channels, dims, or LUTs changed - Update _maybe_refresh() to force full viewer rebuild when structure changes - Update load_dataset() to reset state before loading new dataset This ensures the NDV viewer is properly rebuilt when the data structure changes, rather than just swapping data in-place which leaves stale UI state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use xarray's .sizes.get() instead of .coords.get() for cleaner channel count comparison - Add comprehensive test suite for _data_structure_changed() with 23 test cases covering: - Dimension changes - Channel count changes - Channel name changes - LUT changes - Edge cases (None, empty attrs, no channel dim) - Real-world scenarios (3ch->1ch, live acquisition) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address reviewer feedback: Extract _data_structure_changed from both test classes into a shared module-level data_structure_changed() function to eliminate code duplication and improve maintainability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Args section to _data_structure_changed() docstring - Update comment to list all checks (dims, channels, channel names, LUTs) - Add NOTE in test helper about duplication risk and sync requirement 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove try-except from test helper - test code should fail visibly - Add test_luts_removed_returns_true for symmetry with luts_added test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Prevents resource leaks when switching between datasets by calling _close_open_handles() at the start of load_dataset(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move comparison logic to module-level function for single source of truth - Add dtype change detection (uint8 vs uint16, float vs int) - Update tests to import from ndviewer_light (removes code duplication) - Add 4 new tests: dtype changes (3) and exception propagation (1) - Class method now delegates to module function with exception wrapper This addresses code review feedback: - Eliminates duplicate logic between tests and implementation - Adds missing dtype detection for contrast limit changes - Adds exception path test coverage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Public API method to navigate the viewer to a specific position along any dimension (fov, time, z, channel, etc.). - Encapsulates NDV's display_model.current_index API - Includes fallback for older NDV versions (dims.current_step) - Returns bool for success/failure - Validates dimension exists before setting This enables clean integration when used as a submodule, without callers needing to know NDV internals. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The instance property assignment had no effect - only the type property assignment works for MagicMock objects. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.