🐛 Fix DICOM Reading (Issue #933)

[measty]

In response to issue #933, I tried the dicom image they linked. The reason they couldnt read the pyramid was simply because our DICOMWSIReader expects the folder that contains the levels to be passed as the image path, not the path to an idividual file within the dicom folder. So, their image reads within tiatoolbox just by passing the folder path.

However, while investigating this I found a bug with tiatoolbox dicom reader in that it is not reading at the correct location (wsidicom readregion expects the location in the coordinate space of the resolution being read, and tiatoolbox reader was giving it location in baseline)

This PR fixes that bug (and adds folders that end in .dcm as a valid slide extension for TIAViz so dicom can be viewed in there)

Update - Also updates the parameterized WSIReader tests as follows, to make them more robust so hopefully this sort of bug doesnt slip through again.

All the 'test read consistency' type checks that read regions of the image in different ways that should be equivalent if the reader is working as intended, no longer just read a tile located at (0, 0) which:

1. may not even have any tissue in (making checks like structural similarity meaningless)
2. may not expose bugs related to scaling of location as (0, 0) is a special case in this respect

Instead, these checks now find the center-of-mass of the tissue mask of the WSI, and use a tile located around that instead, to ensure a meaningful tile not located at (0,0) is used. With these changes, the current develop branch will fail the tests, but this bug-fixed branch will pass.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.69%. Comparing base (6ecb9e9) to head (e93cf31).
⚠️ Report is 10 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #934   +/-   ##
========================================
  Coverage    99.69%   99.69%           
========================================
  Files           71       71           
  Lines         8933     8934    +1     
  Branches      1170     1171    +1     
========================================
+ Hits          8906     8907    +1     
  Misses          23       23           
  Partials         4        4           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull Request Overview

This PR fixes a bug in the DICOM reading functionality where the reader was providing baseline coordinates to the read_region API instead of the coordinate‐space appropriate for the requested resolution. It also extends slide discovery to include folders or files with the “.dcm” extension and adds tests to verify that out‐of‐bounds regions correctly return a white image.

• Corrected coordinate usage in DICOMWSIReader so that level_location is passed to read_region.
• Updated slide extension filtering in the Bokeh visualization to include “.dcm”.
• Added tests and fixtures for DICOM samples to validate out‐of‐bounds reads.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
tiatoolbox/wsicore/wsireader.py	Adjusts DICOMWSIReader to pass the correct coordinate (level_location) to read_region to fix bug #933
tiatoolbox/visualization/bokeh_app/main.py	Updates the slide extension list to include “.dcm” so that DICOM folders or files are recognized
tests/test_wsireader.py	Adds a test to ensure that out-of-bound reads return a white (background) image
tests/conftest.py	Introduces a fixture for sample DICOM images to support testing

Comments suppressed due to low confidence (3)

tiatoolbox/visualization/bokeh_app/main.py:727

• Adding '*.dcm' to the extension list allows DICOM files to be correctly recognized as slides. Confirm that this extension handling matches how DICOM-containing directories are structured in your deployment.

for ext in ["*.svs", "*ndpi", "*.tiff", "*.mrxs", "*.jpg", "*.png", "*.tif", "*.dcm"]:

tests/test_wsireader.py:2984

• [nitpick] The new test verifies that out-of-bound DICOM reads return a white background image; consider adding additional edge cases if needed.

def test_oob_read_dicom(sample_dicom: Path) -> None:

tiatoolbox/wsicore/wsireader.py:4842

• Using 'level_location' here ensures that the coordinates are in the correct resolution space as required by the underlying API. Please verify that all related coordinate computations and docstrings reflect this change.

im_region = wsi.read_region(level_location, dicom_level, constrained_read_size)

[fedorov]

The reason they couldnt read the pyramid was simply because our DICOMWSIReader expects the folder that contains the levels to be passed as the image path, not the path to an idividual file within the dicom folder. So, their image reads within tiatoolbox just by passing the folder path.

Thank you for working on this! I thought I tried opening it as a folder, and I thought it didn't work, but I confirm it is working for me as you described. Probably I missed something in my initial testing.

[Copilot]

Pull Request Overview

This PR addresses two main issues: fixing the DICOM reading bug where the wrong coordinate space was used for region reading and adding support for DICOM folders in TIAViz.

• Corrects the incorrect coordinate transformation in DICOMWSIReader by replacing usage of the baseline location with the properly computed level location.
• Updates the file search in the visualization app to include DICOM files by adding the "*.dcm" extension and supplements testing with an out-of-bounds DICOM read.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
tiatoolbox/wsicore/wsireader.py	Fixes coordinate space bug in region reading and updates metadata.
tiatoolbox/visualization/bokeh_app/main.py	Adds *.dcm to recognized slide extensions for TIAViz.
tests/test_wsireader.py	Adds a test for out-of-bounds DICOM region reads.
tests/conftest.py	Introduces a pytest fixture for loading sample DICOM images.

Comments suppressed due to low confidence (2)

tiatoolbox/wsicore/wsireader.py:4598

• [nitpick] Confirm that self.input_path correctly represents the folder path provided for DICOM images. If a more semantically appropriate attribute exists (such as self.input_img), consider using it for clarity.

file_path=self.input_path,

tiatoolbox/wsicore/wsireader.py:4842

• Verify that replacing 'location' with 'level_location' fully resolves the coordinate space inconsistency and aligns with the transformation logic in find_read_rect_params.

im_region = wsi.read_region(level_location, dicom_level, constrained_read_size)

[measty]

This PR is now ready for review/merge

[Copilot]

Pull Request Overview

This PR fixes a bug in the DICOM reader by ensuring that read coordinates are corrected to the appropriate resolution and by accepting folder paths ending in .dcm; it also improves test robustness by using a tissue center‐of-mass based tile for consistency checks.

• Fixes the coordinate space bug in DICOMWSIReader.read_rect
• Extends slide extension support (adds .dcm) in TIAViz and updates tests accordingly
• Improves test functions by using the tissue center‐of-mass for more meaningful comparisons

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tiatoolbox/wsicore/wsireader.py	Fixes coordinate space usage and passes the correct folder path
tiatoolbox/visualization/bokeh_app/main.py	Adds support for DICOM file extensions in slide list population
tests/test_wsireader.py	Replaces (0,0) tile location with a tissue center-of-mass based tile and adds an out-of-bounds DICOM test
tests/conftest.py	Introduces a fixture to fetch a sample DICOM file

[Copilot]

Consider updating the docstring for read_rect to mention that out-of-bounds reads return a white background tile, ensuring consistency with OpenSlide behavior.

[shaneahmed]

Thanks @measty Looks good.

[fedorov]

should this be ".ndpi" - is the dot missing?