Conversation
…for improved clarity and flexibility
…ecord formats in LiveView state
There was a problem hiding this comment.
Pull request overview
This pull request introduces ISO 8601-compliant filename generation for microscopy snapshots and recordings, improving scientific reproducibility and data organization. The changes update both backend filename generation logic and frontend UI to support the new naming convention.
Changes:
- Implemented ISO 8601 timestamp-based filename generation with microsecond precision, detector names, and optional user descriptions
- Added duplicate detector name detection logic to avoid redundant tokens in filenames
- Updated frontend to use sensible defaults (TIFF for snapshots, MP4 for recordings) and improved UI clarity with helper text explaining the new filename format
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| imswitch/imcontrol/controller/controllers/RecordingController.py | Implemented new getFileName method with ISO 8601 timestamps, detector names, and optional descriptions; updated snap method to use new filename generation; changed snapshot folder date format to use hyphens |
| imswitch/imcontrol/model/io/recording_service.py | Added logic to detect and avoid duplicating detector names in generated filepaths |
| frontend/src/components/StreamControls.js | Updated snapshot filename input to be optional with empty default, improved UI labels and helper text, added "(Default)" labels to format options |
| frontend/src/state/slices/LiveViewSlice.js | Added comments clarifying default values for snap and record formats |
| frontend/src/App.jsx | Added useEffect to automatically refresh file list when FileManager view is opened |
| frontend/package.json | Bumped version to 1.5.3 |
| frontend/src/version.js | Bumped version to 1.5.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sx={{ minWidth: 200, flex: 1 }} | ||
| disabled={!isLiveViewActive} | ||
| helperText="Allowed: a-z, A-Z, 0-9, _ - ." | ||
| helperText="Optional description. Filename: ISO8601_DetectorName_Description" |
There was a problem hiding this comment.
The helper text "Optional description. Filename: ISO8601_DetectorName_Description" may be too long for the TextField width and could be truncated or wrap awkwardly on smaller screens. The format described also doesn't quite match the actual implementation which uses hyphens in timestamps (e.g., "2026-02-27T14-32-45-123456").
Consider: 1) shortening the helper text to something like "Optional. Format: timestamp_detector_description", or 2) using a tooltip icon for the full format explanation.
| helperText="Optional description. Filename: ISO8601_DetectorName_Description" | |
| helperText="Optional. Format: timestamp_detector_description" |
| def getFileName(self, description: str = "", detector_names: list = None): | ||
| """ | ||
| Generate a scientifically-meaningful filename with ISO 8601 timestamp. | ||
|
|
||
| Args: | ||
| description: Optional user-provided description (e.g., "cell-005" or "z-stack-start") | ||
| detector_names: List of detector names to include in filename. If provided, uses first detector name. | ||
|
|
||
| Returns: | ||
| Filename in format: YYYY-MM-DDTHH-MM-SS-ffffff_<detector>_<description>.ext | ||
| Example: 2026-02-27T14-32-45-123456_WidefieldCamera_cell-005 | ||
| """ | ||
| # ISO 8601 timestamp with microseconds for uniqueness and sortability | ||
| now = datetime.datetime.now() | ||
| iso_timestamp = now.strftime("%Y-%m-%dT%H-%M-%S") | ||
| microseconds = f"{now.microsecond:06d}" | ||
| timestamp = f"{iso_timestamp}-{microseconds}" | ||
|
|
||
| # Add detector name if available | ||
| detector_part = "" | ||
| if detector_names and len(detector_names) > 0: | ||
| # Use first detector name (sanitize it to be filename-safe) | ||
| detector_name = detector_names[0] | ||
| # Remove special characters that could cause issues | ||
| safe_detector = detector_name.replace(" ", "_").replace("/", "_") | ||
| detector_part = f"_{safe_detector}" | ||
|
|
||
| # Add description if provided (sanitize it) | ||
| description_part = "" | ||
| if description and description.strip(): | ||
| # Sanitize description | ||
| safe_description = description.strip().replace(" ", "_").replace("/", "_") | ||
| description_part = f"_{safe_description}" | ||
|
|
||
| # Combine all parts | ||
| filename = f"{timestamp}{detector_part}{description_part}" | ||
| return filename |
There was a problem hiding this comment.
The new getFileName method with its ISO 8601 timestamp generation, sanitization logic, and detector name handling does not have any test coverage. Given that this is a critical function for scientific reproducibility (as mentioned in the PR description) and generates filenames used throughout the recording system, it should have unit tests to verify:
- Correct ISO 8601 timestamp format generation
- Proper sanitization of special characters in detector names and descriptions
- Correct handling of empty/None values for optional parameters
- Filename uniqueness (microsecond precision)
Consider adding tests in a file like imswitch/imcontrol/_test/unit/test_recording_controller.py or similar.
imswitch/imcontrol/controller/controllers/RecordingController.py
Outdated
Show resolved
Hide resolved
| def getFileName(self): | ||
| """Gets the filename of the data to save.""" | ||
| filename = time.strftime("%Hh%Mm%Ss") | ||
| def getFileName(self, description: str = "", detector_names: list = None): |
There was a problem hiding this comment.
The type hint for detector_names parameter is list but should be List[str] or list[str] for better type safety and clarity. This is inconsistent with Python typing conventions and could be confusing for developers. Consider using the proper type hint to indicate that it's a list of strings, not just any list.
| def getFileName(self, description: str = "", detector_names: list = None): | |
| def getFileName(self, description: str = "", detector_names: Optional[List[str]] = None): |
| if name is None: | ||
| name = "_snap" | ||
| savename = os.path.join(folder, self.getFileName() + "_" + name) | ||
| name = "" | ||
| savename = os.path.join(folder, self.getFileName(description=name, detector_names=detectorNames)) |
There was a problem hiding this comment.
The snap method now passes the user description directly to getFileName via the name parameter (line 205). However, when name is None, it's converted to an empty string (line 204). This is appropriate for the new filename format where the description is optional and the timestamp+detector name provide the core identification.
Note: The returned fullPath won't include the file extension that gets added later by recording_service._generate_filepath, which could cause confusion for API consumers expecting complete file paths.
| # Remove special characters that could cause issues | ||
| safe_detector = detector_name.replace(" ", "_").replace("/", "_") | ||
| detector_part = f"_{safe_detector}" | ||
|
|
||
| # Add description if provided (sanitize it) | ||
| description_part = "" | ||
| if description and description.strip(): | ||
| # Sanitize description | ||
| safe_description = description.strip().replace(" ", "_").replace("/", "_") | ||
| description_part = f"_{safe_description}" |
There was a problem hiding this comment.
The sanitization logic replaces spaces and forward slashes with underscores, but doesn't handle other potentially problematic filename characters like backslashes, colons, asterisks, question marks, quotes, pipes, etc. These characters can cause issues on different operating systems (especially Windows).
Consider using a more comprehensive sanitization approach that either uses a whitelist of allowed characters or replaces a broader set of filesystem-unsafe characters. For example: re.sub(r'[<>:"/\\|?*]', '_', text) or similar.
| def getFileName(self, description: str = "", detector_names: list = None): | ||
| """ | ||
| Generate a scientifically-meaningful filename with ISO 8601 timestamp. | ||
|
|
||
| Args: | ||
| description: Optional user-provided description (e.g., "cell-005" or "z-stack-start") | ||
| detector_names: List of detector names to include in filename. If provided, uses first detector name. | ||
|
|
||
| Returns: | ||
| Filename in format: YYYY-MM-DDTHH-MM-SS-ffffff_<detector>_<description>.ext | ||
| Example: 2026-02-27T14-32-45-123456_WidefieldCamera_cell-005 | ||
| """ | ||
| # ISO 8601 timestamp with microseconds for uniqueness and sortability | ||
| now = datetime.datetime.now() | ||
| iso_timestamp = now.strftime("%Y-%m-%dT%H-%M-%S") | ||
| microseconds = f"{now.microsecond:06d}" | ||
| timestamp = f"{iso_timestamp}-{microseconds}" | ||
|
|
||
| # Add detector name if available | ||
| detector_part = "" | ||
| if detector_names and len(detector_names) > 0: | ||
| # Use first detector name (sanitize it to be filename-safe) | ||
| detector_name = detector_names[0] | ||
| # Remove special characters that could cause issues | ||
| safe_detector = detector_name.replace(" ", "_").replace("/", "_") | ||
| detector_part = f"_{safe_detector}" | ||
|
|
||
| # Add description if provided (sanitize it) | ||
| description_part = "" | ||
| if description and description.strip(): | ||
| # Sanitize description | ||
| safe_description = description.strip().replace(" ", "_").replace("/", "_") | ||
| description_part = f"_{safe_description}" | ||
|
|
||
| # Combine all parts | ||
| filename = f"{timestamp}{detector_part}{description_part}" | ||
| return filename |
There was a problem hiding this comment.
The updated getFileName method now has a new signature that accepts description and detector_names parameters. However, there's a call to getFileName() at line 565 (in the startRecording method, outside this diff) that doesn't pass these parameters. This will result in recording filenames that lack detector information and are inconsistent with the new snapshot filename format.
To maintain consistency between snapshots and recordings, the startRecording method should be updated to pass the detector names when calling getFileName.
| detector_token = f"_{detector_name}" | ||
| if detector_token in base_name: |
There was a problem hiding this comment.
The duplicate detector name detection logic uses simple substring matching (if detector_token in base_name), which can produce false positives. For example, if the detector is named "Camera" and the description is "CameraTest", this would incorrectly skip adding the detector name.
Consider using a more precise matching approach, such as checking if the detector token appears as a complete underscore-separated segment rather than as a substring.
| detector_token = f"_{detector_name}" | |
| if detector_token in base_name: | |
| # Check if the detector name already appears as a full underscore-separated segment | |
| base_name_segments = base_name.split('_') | |
| if detector_name in base_name_segments: |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ions in LiveView and RecordingController
…gController and RecordingService
…CapturePath for consistency in LiveView and StreamControls components
…d optimize initialPath handling in context
…ng images with improved file path handling
…e path handling in RecordingService to avoid metadata issues
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
imswitch/imcontrol/controller/controllers/RecordingController.py
Outdated
Show resolved
Hide resolved
| def getFileName(self, description: str = "", detector_names: list = None): | ||
| """ | ||
| Generate a scientifically-meaningful filename with ISO 8601 timestamp. | ||
|
|
||
| Args: | ||
| description: Optional user-provided description (e.g., "cell-005" or "z-stack-start") | ||
| detector_names: List of detector names to include in filename. If provided, uses first detector name. | ||
|
|
||
| Returns: | ||
| Filename in format: YYYY-MM-DDTHH-MM-SS-ffffff_<detector>_<description> (without extension) | ||
| Example (basename only): 2026-02-27T14-32-45-123456_WidefieldCamera_cell-005 | ||
| """ | ||
| # ISO 8601 timestamp with microseconds for uniqueness and sortability | ||
| now = datetime.datetime.now() | ||
| iso_timestamp = now.strftime("%Y-%m-%dT%H-%M-%S") | ||
| microseconds = f"{now.microsecond:06d}" | ||
| timestamp = f"{iso_timestamp}-{microseconds}" | ||
|
|
||
| # Add detector name if available | ||
| detector_part = "" | ||
| if detector_names and len(detector_names) > 0: | ||
| # Use first detector name (sanitize it to be filename-safe) | ||
| detector_name = detector_names[0] | ||
| # Remove special characters that could cause issues | ||
| safe_detector = detector_name.replace(" ", "_").replace("/", "_") | ||
| detector_part = f"_{safe_detector}" | ||
|
|
||
| # Add description if provided (sanitize it) | ||
| description_part = "" | ||
| if description and description.strip(): | ||
| # Sanitize description | ||
| safe_description = description.strip().replace(" ", "_").replace("/", "_") | ||
| description_part = f"_{safe_description}" |
There was a problem hiding this comment.
getFileName() claims to generate an “ISO 8601 timestamp”, but the output uses filename-safe separators (e.g., T%H-%M-%S-ffffff) and no timezone offset. Either update the docstring/examples to match the actual (filename-safe) format, or switch to a true ISO-8601 representation (and then sanitize it for filenames). Also, detector/description sanitization currently only replaces spaces and /; characters like : and \ can still appear (problematic on Windows). Consider using a stricter whitelist or a shared filename-sanitizer helper.
… RecordingController and RecordingService
This pull request introduces improvements to file naming conventions for snapshots and recordings, updates the frontend UI to clarify filename formatting, and ensures sensible defaults for file formats. The changes enhance scientific reproducibility and user experience by generating ISO 8601-compliant, descriptive filenames and making the UI more intuitive.
File naming improvements:
getFileNamemethod inRecordingController.pynow generates filenames in the formatYYYY-MM-DDTHH-MM-SS-ffffff_<detector>_<description>, using ISO 8601 timestamps and including detector and optional user descriptions for clarity and uniqueness.getFileName, resulting in filenames that better reflect the experiment context._generate_filepathmethod inrecording_service.pyavoids duplicating detector names in filenames if already present, preventing redundant tokens.Frontend UI and default handling:
TIFF) and recordings (MP4), both in Redux state and UI, ensuring consistency and clarity for users. [1] [2]Version bump:
1.5.3in bothpackage.jsonandversion.jsto reflect these changes. [1] [2]Minor fixes: