Skip to content
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.

Sign up for GitHub

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

Jump to bottom

🐛 Fix TIFFWSIReader read_bound #777

Merged
merged 27 commits into from
Jul 5, 2024
Merged

🐛 Fix TIFFWSIReader read_bound #777

merged 27 commits into from
Jul 5, 2024

Conversation

vqdang
Copy link
Contributor

@vqdang vqdang commented Jan 29, 2024 •
edited by shaneahmed
Loading

image

Emergency bugfix per @John-P request.
The culprit is reading bound doesn't use the adjusted bounds as have been done in OpenSlideReader.

@shaneahmed shaneahmed added the bug Something isn't working label Feb 2, 2024
@shaneahmed shaneahmed added this to the Release v1.5.2 milestone Feb 2, 2024
@shaneahmed shaneahmed changed the title BUG: Fix TIFFWSIReader read bound 🐛 Fix TIFFWSIReader read_bound Feb 2, 2024
@shaneahmed
Copy link
Member

Thanks @vqdang and @John-P Please can you add a simple test to make sure this produces expected results.

@codecov Codecov
Copy link

codecov bot commented Feb 2, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.86%. Comparing base (621a857) to head (a2c8afe).
Report is 1 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #777      +/-   ##
===========================================
- Coverage    99.89%   99.86%   -0.04%     
===========================================
  Files           69       69              
  Lines         8650     8650              
  Branches      1653     1654       +1     
===========================================
- Hits          8641     8638       -3     
- Misses           1        4       +3     
  Partials         8        8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shaneahmed shaneahmed linked an issue Feb 2, 2024 that may be closed by this pull request
Issues with TIFFWSIReader #499
Closed
@shaneahmed
Copy link
Member

Please can you also check if it is related to #499 ?

@Abdol
Copy link
Collaborator

Abdol commented May 13, 2024

I have added a simple test but not completely sure if it fully tests for the fixed bug. @vqdang @John-P I'd appreciate it if you can give the test a review.

@Abdol Abdol self-assigned this May 14, 2024
@Abdol Abdol requested a review from John-P May 14, 2024 11:02
@Abdol
Copy link
Collaborator

Abdol commented Jun 7, 2024

A kind follow up @vqdang @John-P. I'd appreciate it if you can give the test a review. Thanks!

@measty
Copy link
Collaborator

measty commented Jun 24, 2024

I think this also needs a fix along similar lines for read_rect

@Abdol
Copy link
Collaborator

Abdol commented Jun 25, 2024

A kind follow up @vqdang @John-P. I'd appreciate it if you can give the test a review. Thank you!

@Abdol Abdol added the help wanted Extra attention is needed label Jun 26, 2024
@Abdol Abdol requested review from measty and mostafajahanifar June 26, 2024 09:31
@measty
Copy link
Collaborator

measty commented Jun 27, 2024

I've added a few changes to this PR too.

  • fix for read_rect along similar lines to read_bounds (it was bugged in the same way)
  • a few changes to make the reader compatible with a wider variety of tiffs
  • a change to metadata in wsireader that fixes some performance issues on some wsis

This was referenced Jun 28, 2024
Open
Closed
@shaneahmed shaneahmed linked an issue Jun 28, 2024 that may be closed by this pull request
Get ValueError: Unsupported axes YX when using OME-TIFF for nuclear segmentation #509
Closed
@Abdol
Copy link
Collaborator

Abdol commented Jun 28, 2024

After updating the sample OME TIFF, the bug can be reproduced using the existing level consistency tests. So, I have removed the newly-added test. This PR should be good to go now.

@Abdol Abdol requested review from shaneahmed and removed request for John-P and mostafajahanifar June 28, 2024 15:01
shaneahmed
shaneahmed approved these changes Jul 5, 2024
View reviewed changes
Copy link
Member

@shaneahmed shaneahmed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Abdol , @measty and @vqdang for your help in fixing this bug.

@shaneahmed shaneahmed merged commit 82e9d8f into develop Jul 5, 2024
2 of 3 checks passed
@shaneahmed shaneahmed deleted the bugfix-tiffwsireader-readbounds branch July 5, 2024 11:43
@shaneahmed shaneahmed mentioned this pull request Dec 10, 2024
Merged
shaneahmed added a commit that referenced this pull request Dec 12, 2024
@shaneahmed @John-P
@pre-commit-ci @adamshephard @measty @mostafajahanifar @simongraham @Jiaqi-Lv @behnazelhaminia @GeorgeBatch @vqdang @Alon-Alexander
🔖 Release 1.6.0 (#895)
e14a4fc 
## TIAToolbox v1.6.0 (2024-12-12)

### Major Updates and Feature Improvements

- **Foundation Models Support via `timm` API** (#856, contributed by @GeorgeBatch)
  - Introduced `TimmBackbone` for running additional PyTorch Image Models.
  - Tested models include `UNI`, `Prov-GigaPath`, and `H-optimus-0`.
  - Added an example notebook demonstrating feature extraction with foundation models.
  - `timm` added as a dependency.
- **Performance Enhancements with `torch.compile`** (#716)
  - Improved performance on newer GPUs using `torch.compile`.
- **Multichannel Input Support in `WSIReader`** (#742)
- **AnnotationStore Filtering for Patch Extraction** (#822)
- **Python 3.12 Support**
- **Deprecation of Python 3.8 Support**
- **CLI Response Time Improvements** (#795)

### API Changes

- **Device Specification Update** (#882)
  - Replaced `has_gpu` with `device` for specifying GPU or CPU usage, aligning with PyTorch's `Model.to()` functionality.
- **Windows Compatibility Enhancement** (#769)
  - Replaced `POWER` with explicit multiplication.

### Bug Fixes and Other Changes

- **TIFFWSIReader Bound Reading Adjustment** (#777)
  - Fixed `read_bound` to use adjusted bounds.
  - Reduced code complexity in `WSIReader` (#814).
- **Annotation Rendering Fixes** (#813)
  - Corrected rendering of annotations with holes.
- **Non-Tiled TIFF Support in `WSIReader`** (#807, contributed by @GeorgeBatch)
- **HoVer-Net Documentation Update** (#751)
  - Corrected class output information.
- **Citation File Fix for `cffconvert`** (#869, contributed by @Alon-Alexander)
- **Bokeh Compatibility Updates**
  - Updated `bokeh_app` for compatibility with `bokeh>=3.5.0`.
  - Switched from `size` to `radius` for `bokeh>3.4.0` compatibility (#796).
- **JSON Extraction Fixes** (#772)
  - Restructured SQL expression construction for JSON properties with dots in keys.
- **VahadaneExtractor Warning** (#871)
  - Added warning due to changes in `scikit-learn>0.23.0` dictionary learning (#382).
- **PatchExtractor Error Message Refinement** (#883)
- **Immutable Output Fix in `WSIReader`** (#850)

### Development-Related Changes

- **Mypy Checks Added**
  - Applied to `utils`, `tools`, `data`, `annotation`, and `cli/common`.
- **ReadTheDocs PDF Build Deprecation**
- **Formatter Update**
  - Replaced `black` with `ruff-format`.
- **Dependency Removal**
  - Removed `jinja2`.
- **Test Environment Update**
  - Updated to `Ubuntu 24.04`.
- **Conda Environment Workflow Update**
  - Implemented `micromamba` setup.
- **Codecov Reporting Fix** (#811)
  **Full Changelog:** v1.5.1...v1.6.0

---------

Co-authored-by: John Pocock <John-P@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Adam Shephard <39619155+adamshephard@users.noreply.github.com>
Co-authored-by: Mark Eastwood <20169086+measty@users.noreply.github.com>
Co-authored-by: Mostafa Jahanifar <74412979+mostafajahanifar@users.noreply.github.com>
Co-authored-by: Simon Graham <20071401+simongraham@users.noreply.github.com>
Co-authored-by: Abdol A <u2271662@live.warwick.ac.uk>
Co-authored-by: Jiaqi-Lv <60471431+Jiaqi-Lv@users.noreply.github.com>
Co-authored-by: Dmitrii Blaginin <blaginin@mbp.lan>
Co-authored-by: behnazelhaminia <30952176+behnazelhaminia@users.noreply.github.com>
Co-authored-by: George Batchkala <46561186+GeorgeBatch@users.noreply.github.com>
Co-authored-by: vqdang <24943262+vqdang@users.noreply.github.com>
Co-authored-by: Jiaqi Lv <lvjiaqi9@gmail.com>
Co-authored-by: Alon Alexander <alon008@gmail.com>
@shaneahmed shaneahmed mentioned this pull request Dec 12, 2024
Merged
shaneahmed added a commit that referenced this pull request Dec 12, 2024
@shaneahmed
🔖 Release 1.6.0 (#898)
0ddd2b1 
## TIAToolbox v1.6.0 (2024-12-12)

### Major Updates and Feature Improvements

- **Foundation Models Support via `timm` API** (#856, contributed by @GeorgeBatch)
  - Introduced `TimmBackbone` for running additional PyTorch Image Models.
  - Tested models include `UNI`, `Prov-GigaPath`, and `H-optimus-0`.
  - Added an example notebook demonstrating feature extraction with foundation models.
  - `timm` added as a dependency.
- **Performance Enhancements with `torch.compile`** (#716)
  - Improved performance on newer GPUs using `torch.compile`.
- **Multichannel Input Support in `WSIReader`** (#742)
- **AnnotationStore Filtering for Patch Extraction** (#822)
- **Python 3.12 Support**
- **Deprecation of Python 3.8 Support**
- **CLI Response Time Improvements** (#795)

### API Changes

- **Device Specification Update** (#882)
  - Replaced `has_gpu` with `device` for specifying GPU or CPU usage, aligning with PyTorch's `Model.to()` functionality.
- **Windows Compatibility Enhancement** (#769)
  - Replaced `POWER` with explicit multiplication.

### Bug Fixes and Other Changes

- **TIFFWSIReader Bound Reading Adjustment** (#777)
  - Fixed `read_bound` to use adjusted bounds.
  - Reduced code complexity in `WSIReader` (#814).
- **Annotation Rendering Fixes** (#813)
  - Corrected rendering of annotations with holes.
- **Non-Tiled TIFF Support in `WSIReader`** (#807, contributed by @GeorgeBatch)
- **HoVer-Net Documentation Update** (#751)
  - Corrected class output information.
- **Citation File Fix for `cffconvert`** (#869, contributed by @Alon-Alexander)
- **Bokeh Compatibility Updates**
  - Updated `bokeh_app` for compatibility with `bokeh>=3.5.0`.
  - Switched from `size` to `radius` for `bokeh>3.4.0` compatibility (#796).
- **JSON Extraction Fixes** (#772)
  - Restructured SQL expression construction for JSON properties with dots in keys.
- **VahadaneExtractor Warning** (#871)
  - Added warning due to changes in `scikit-learn>0.23.0` dictionary learning (#382).
- **PatchExtractor Error Message Refinement** (#883)
- **Immutable Output Fix in `WSIReader`** (#850)

### Development-Related Changes

- **Mypy Checks Added**
  - Applied to `utils`, `tools`, `data`, `annotation`, and `cli/common`.
- **ReadTheDocs PDF Build Deprecation**
- **Formatter Update**
  - Replaced `black` with `ruff-format`.
- **Dependency Removal**
  - Removed `jinja2`.
- **Test Environment Update**
  - Updated to `Ubuntu 24.04`.
- **Conda Environment Workflow Update**
  - Implemented `micromamba` setup.
- **Codecov Reporting Fix** (#811)
  **Full Changelog:** v1.5.1...v1.6.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shaneahmed shaneahmed shaneahmed approved these changes

@measty measty Awaiting requested review from measty

Assignees

@Abdol Abdol

@vqdang vqdang

Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Milestone
Release v1.5.2
Development

Successfully merging this pull request may close these issues.

Get ValueError: Unsupported axes YX when using OME-TIFF for nuclear segmentation Issues with TIFFWSIReader
4 participants
@vqdang @shaneahmed @Abdol @measty