Skip to content

Conversation

@measty
Copy link
Collaborator

@measty measty commented Dec 8, 2023

@codecov
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c3b49e6) 99.91% compared to head (934faff) 99.91%.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop     #752    +/-   ##
=========================================
  Coverage    99.91%   99.91%            
=========================================
  Files           69       69            
  Lines         8507     8511     +4     
  Branches      1379     1636   +257     
=========================================
+ Hits          8500     8504     +4     
  Partials         7        7            

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

@shaneahmed shaneahmed changed the title fix tile save 🩹 Fix save_tiles Dec 11, 2023
@shaneahmed shaneahmed added this to the Release v1.5.0 milestone Dec 11, 2023
@shaneahmed shaneahmed linked an issue Dec 11, 2023 that may be closed by this pull request
@shaneahmed shaneahmed requested a review from Abdol December 11, 2023 13:32
@shaneahmed shaneahmed added the bug Something isn't working label Dec 11, 2023
Copy link
Collaborator

@Abdol Abdol left a comment

Choose a reason for hiding this comment

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

Running the code on a couple of WSIs from the TUPAC dataset, I can say that the produced the tiles are so far consistent in size except where tiles are generated at the edges of the WSI.

From this first test, I think the changes are valid. If needed, I can run further tests on more WSIs with additional parameter/option tweaking to verify further.

Here is the code I used for testing:

import os
from tiatoolbox.wsicore.wsireader import WSIReader

obj_pwrs = [5, 10, 20]
for obj_pwr in obj_pwrs:
    # cycle through all the images in the WSI folder
    for wsi_file in os.listdir("wsi"):
        if '.DS_Store' in wsi_file:
            continue
        path = "wsi/" + wsi_file
        wsi = WSIReader.open(input_img=path)
        print(wsi.info.as_dict())
        wsi.save_tiles(output_dir="tiles-" + str(obj_pwr) + "x", tile_objective_value=obj_pwr, tile_read_size=(1024,1024), verbose=True)

The data I used can be downloaded here (TUPAC-TR-001.svs and TUPAC-TR-002.svs).

I also attach some of the produced CSVs:
output-TUPAC-TR-001-5x.csv
output-TUPAC-TR-001-10x.csv
output-TUPAC-TR-001-20x.csv

Thank you @measty

@Abdol
Copy link
Collaborator

Abdol commented Dec 12, 2023

I ran more tests on 8 WSIs from OpenSlide samples and 5 from the ACROBAT dataset and I don't see any issues, except that the size of tiles becomes inconsistent at the edges of the WSI, e.g., instead of (1024, 1024) for a tile, you can get (x, 1024) or (x, y) with x and y being integer numbers less than 1024.

Here are some CSVs that showcase that (scroll to the bottom):
CMU-3.ndpi-5x.csv
CMU-3.svs-20x.csv
JP2K-33003-1.svs-10x.csv

If that's expected behaviour, then feel free to close this PR.

@measty
Copy link
Collaborator Author

measty commented Dec 12, 2023

I ran more tests on 8 WSIs from OpenSlide samples and 5 from the ACROBAT dataset and I don't see any issues, except that the size of tiles becomes inconsistent at the edges of the WSI, e.g., instead of (1024, 1024) for a tile, you can get (x, 1024) or (x, y) with x and y being integer numbers less than 1024.

Here are some CSVs that showcase that (scroll to the bottom): CMU-3.ndpi-5x.csv CMU-3.svs-20x.csv JP2K-33003-1.svs-10x.csv

If that's expected behaviour, then feel free to close this PR.

As far as I know, that is the expected behaviour. Certainly that is how it behaved before on the slides where it was working, so in this PR I have not changed that, I have just fixed the behaviour on the slides where it was returning nonesense tiles at wrong resolutions.

If it is wanted that we change the behavior to return padded tiles near edges, it is a simple change; we just have to remove the lines:

            end_h = min(end_h, slide_h)
            end_w = min(end_w, slide_w)

@shaneahmed shaneahmed merged commit 4edd779 into develop Dec 15, 2023
@shaneahmed shaneahmed deleted the fix-save-tiles branch December 15, 2023 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WSIReader Tile Sizes Issue - SVS Format

4 participants