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.

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

WIP: Add FsspecJsonWSIReader class. #897

Draft
wants to merge 30 commits into
base: develop
Choose a base branch
from

Conversation

aacic
Copy link
Collaborator

@aacic aacic commented Dec 9, 2024

The FsspecJsonWSIReader reads fsspec json file which represents SVS wsi.
The methods: read_rect, read_bounds, _canonical_shape are copied from TIFFWSIReader, and it needs to be improved by introducing delegates or some other way.

The method infor reads SVS metadata which is stored in the root group metadata like:

{
  ".zattrs": {
    "multiscales": [
      {
        "metadata": {
          "objective_power": 40,
          "vendor": "Aperio",
          "mpp": [0.2525, 0.2525]	
        }
      }
    ]
  }
}

To test, execute from the root dir:

pip install -r requirements/requirements_dev.txt
mkdir -p samples/slides
mkdir -p samples/fsspec
cd samples/slides
curl -C - -o TCGA-22-1017-01Z-00-DX1.9562FE79-A261-42D3-B394-F3E0E2FF7DDA.svs   https://api.gdc.cancer.gov/data/73c69d24-6f9e-44e2-bfe5-a608d4cf5c27
cd ../../tests/zarrtiff/
python tiff_fsspec.py "../../samples/slides/TCGA-22-1017-01Z-00-DX1.9562FE79-A261-42D3-B394-F3E0E2FF7DDA.svs"  "../../samples/fsspec/73c69d24-6f9e-44e2-bfe5-a608d4cf5c27_fsspec.json" "https://api.gdc.cancer.gov/data/73c69d24-6f9e-44e2-bfe5-a608d4cf5c27"

Change the svs variable inside of tileserver.py to:

svs = "../../samples/fsspec/73c69d24-6f9e-44e2-bfe5-a608d4cf5c27_fsspec.json"

python tileserver.py

Open http://127.0.0.1:5000/ and verify how it works.

@aacic aacic changed the title Add ZarrTIFFWSIReader class. WIP: Add ZarrTIFFWSIReader class. Dec 9, 2024
@aacic aacic force-pushed the zarr-tiff-wsi-reader branch from 3805565 to cbd657f Compare December 12, 2024 13:59
@aacic aacic force-pushed the zarr-tiff-wsi-reader branch from bfa8b4b to 1bc2356 Compare December 16, 2024 16:51
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

codecov bot commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 72.58065% with 51 lines in your changes missing coverage. Please review.

Project coverage is 99.34%. Comparing base (d82df5c) to head (1ce4610).

Files with missing lines Patch % Lines
tiatoolbox/wsicore/wsireader.py 66.66% 46 Missing and 3 partials ⚠️
tiatoolbox/utils/tiff_to_fsspec.py 94.87% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #897      +/-   ##
===========================================
- Coverage    99.90%   99.34%   -0.57%     
===========================================
  Files           70       71       +1     
  Lines         8735     8852     +117     
  Branches      1149     1163      +14     
===========================================
+ Hits          8727     8794      +67     
- Misses           3       50      +47     
- Partials         5        8       +3     

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

Args:
path (Path): Path to the file to check.
# TODO extend logic and verify that json file is a fsspec tiff file
Copy link
Member

Choose a reason for hiding this comment

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

Add a link to tiff-fsspec generator file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The method is migrated to:
FsspecJsonWSIReader.is_valid_zarr_fsspec(file_path: str) -> bool:

I've extended the docs and added a link to tiff-to-fsspec generator file.

@@ -4225,6 +4246,528 @@ class docstrings for more information.
return im_region


class ZarrTIFFWSIReader(WSIReader):
Copy link
Member

@shaneahmed shaneahmed Jan 14, 2025

Choose a reason for hiding this comment

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

Please change to appropriate name e.g., fsspecWSIReader.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to FsspecJsonWSIReader.

@@ -4225,6 +4246,528 @@ class docstrings for more information.
return im_region


class ZarrTIFFWSIReader(WSIReader):
"""Define Zarr Tiff WSI Reader."""
Copy link
Member

Choose a reason for hiding this comment

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

Add some documentation / introduction about the fsspec / json files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added documentation and the link to tiff_to_fsspec.py.

@aacic aacic mentioned this pull request Jan 23, 2025
3 tasks
@shaneahmed shaneahmed added the enhancement New feature or request label Jan 24, 2025
@shaneahmed shaneahmed added this to the Release v1.7.0 milestone Jan 24, 2025
@@ -0,0 +1,147 @@
"""Module for processing SVS metadata and generating fsspec JSON file."""
Copy link
Member

Choose a reason for hiding this comment

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

It would be useful to add some description about why we need fsspec JSON file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've included a more detailed description and cleaned up the script.

I've working to bring the PR to the state when it's "Ready for review". It will require some more effort.

@aacic aacic changed the title WIP: Add ZarrTIFFWSIReader class. WIP: Add FsspecJsonWSIReader class. Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants