Conversation
kmitcham
requested review from
BrianWhitneyAI,
SeanDuHare,
aswallace,
Copilot,
pgarrison and
toloudis
and removed request for
BrianWhitneyAI,
SeanDuHare,
aswallace,
pgarrison and
toloudis
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #13 by unpinning several dependencies and improving error handling for non-tiled TIFF files. The changes ensure the reader properly validates file format compatibility and gracefully handles missing TIFF tag structures.
- Unpinned dependency versions for
bfio,dask, andtifffileto allow more flexibility - Added validation to reject non-tiled TIFF formats early in the reader initialization
- Improved TIFF tag extraction with exception handling to prevent crashes when tags are unavailable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pyproject.toml | Removed version pins from bfio and dask dependencies; updated tifffile version constraints with Python version-specific requirements |
| bioio_ome_tiled_tiff/reader.py | Added validation to ensure only tiled-tiff format is processed; refactored TIFF tag extraction with exception handling; reformatted import statements and documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kmitcham
requested review from
BrianWhitneyAI,
SeanDuHare,
aswallace and
pgarrison
| try: | ||
| self._rdr = BioReader(self._path, backend="python") | ||
| # limit reader to only tiled images | ||
| if self._rdr._backend_name != "python": |
There was a problem hiding this comment.
So the python backend is the one that supports tiled reading? does it support other things that have changed? Essentially Im asking is this restriction exact?
There was a problem hiding this comment.
The python backend is the one that does tiled reading. I don't know what it does allow, but it blocks lif and ome.tiff
There was a problem hiding this comment.
I might opt to try for a tighter assessment here. Do I know that's possible? No
bioio_ome_tiled_tiff/reader.py
Outdated
| _current_scene_index: int = 0 | ||
| # Do not provide default value because | ||
| # they may not need to be used by your reader (i.e. input param is an array) | ||
| # they may not need to be used by your reader |
There was a problem hiding this comment.
what is this comment actually saying? Wouldn't defaults be overwritten by new params regardless?
| self._rdr = BioReader(self._path, backend="python") | ||
| # limit reader to only tiled images | ||
| if self._rdr._backend_name != "python": | ||
| # while bfio supports other formats, bio has specialized |
There was a problem hiding this comment.
Feels like this comment would be useful to the user
3 participants
Link to Relevant Issue
#13
Description of Changes
Update code to catch an exception if underlying structures are not present
Update reader to fail on files that are NOT tiled-tif format