Conversation
There was a problem hiding this comment.
Pull request overview
Adds first-class support for common raster image formats (PNG/JPEG/BMP/GIF) alongside TIFF, improves virtual-workspace extension filtering to be case-insensitive, and corrects a misleading missing-reader/writer error message.
Changes:
- Introduce
FileFormat.RASTER_IMAGEand include its extensions inDEFAULT_IMAGE_EXTENSIONS. - Register raster image read/write handlers in the disk backend using
imageio.v3. - Make
VirtualWorkspace.list_files()extension filtering case-insensitive and fix the disk-backend “missing reader” error message.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/polystore/virtual_workspace.py |
Lowercase-normalizes extension filters and suffixes for case-insensitive matching in list_files(). |
src/polystore/formats.py |
Adds RASTER_IMAGE format + extensions; updates DEFAULT_IMAGE_EXTENSIONS to include both TIFF and raster formats. |
src/polystore/disk.py |
Switches optional import to importlib, adds imageio integration, registers raster handlers, and fixes the missing-reader error message. |
pyproject.toml |
Adds imageio as a dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _image_reader(self, path): | ||
| """Read standard raster images using imageio.""" | ||
| return imageio.imread(path) |
There was a problem hiding this comment.
DiskStorageBackend.load() calls the registered reader as reader(disk_path, **kwargs). The new _image_reader does not accept **kwargs, so any non-empty kwargs (even benign ones) will raise a TypeError when loading raster images. Please update _image_reader to accept **kwargs (and either ignore them or pass supported options through to imageio.imread).
| def _image_reader(self, path): | |
| """Read standard raster images using imageio.""" | |
| return imageio.imread(path) | |
| def _image_reader(self, path, **kwargs): | |
| """Read standard raster images using imageio.""" | |
| return imageio.imread(path, **kwargs) |
| def _image_writer(self, path, data, **kwargs): | ||
| """Write standard raster images using imageio.""" | ||
| imageio.imwrite(path, np.asarray(data)) | ||
|
|
||
| def _image_reader(self, path): |
There was a problem hiding this comment.
Raster image support is introduced here, but the test suite doesn’t cover saving/loading any of the new extensions (e.g., .png/.jpg/.bmp). Please add pytest coverage that round-trips a small array through at least one raster format and asserts the extension is registered/usable (and ideally verifies case-insensitive extension handling, e.g., '.PNG').
| # Apply filters | ||
| vpath = Path(virtual_relative) | ||
| if pattern and not fnmatch(vpath.name, pattern): | ||
| continue | ||
| if extensions and vpath.suffix not in extensions: | ||
| if lowercase_extensions and vpath.suffix.lower() not in lowercase_extensions: |
There was a problem hiding this comment.
This makes extension filtering case-insensitive for virtual workspace listings, but there are no tests covering VirtualWorkspace.list_files() extension filtering. Please add a focused test with mixed-case suffixes (e.g., '.PNG') and assert extensions={'.png'} still matches.
Adds a typed raster-image format family for PNG/JPEG/BMP/GIF via imageio, case-insensitive virtual workspace extension matching, and fixes the missing-reader error message. This is required by OpenHCS CellProfiler ExampleColocalization execution, which uses PNG inputs.