:bug: BUG: Fix Level When Using DICOM `read_region` by measty · Pull Request #564 · TissueImageAnalytics/tiatoolbox

measty · 2023-03-10T19:18:05Z

This PR fixes the dicom slide thunbnail issue (maybe). the issue seems to arise from a difference in how the wsidicom reader treats levels, in that it labels them by their relative magnification (in power of 2).

so for example the levels in the wsidicom reader are [0, 2, 4] if the level downsamples are [1, 4, 16]

whereas in our WSIReader level is just an index [0, 1, 2], and the level_downsamples are separate.

My knowledge of DICOM is nonexistant, so it may be that the fix is wrong - hopefully john can have a look at it. It fixed the thumbnail issue for me though.

codecov · 2023-03-10T19:47:43Z

Codecov Report

Merging #564 (b2afb5b) into develop (8ce0ebf) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop     #564   +/-   ##
========================================
  Coverage    99.51%   99.51%           
========================================
  Files           62       62           
  Lines         6591     6596    +5     
  Branches      1053     1078   +25     
========================================
+ Hits          6559     6564    +5     
  Misses          20       20           
  Partials        12       12

Impacted Files	Coverage Δ
tiatoolbox/wsicore/wsireader.py	`98.48% <100.00%> (+<0.01%)`	⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

measty · 2023-03-16T13:07:44Z

I've tested this on another example that john provided (a non-folder dicom), and it worked as expected, so i'm fairly confident this is ok, now its been tested on more than one example.

John-P · 2023-03-17T00:00:31Z

I can't find where in the wsidicom source or documentation this behaviour is specified. Can you provide any reference? It's not really a DICOM thing but a wsidicom thing IMO.

Perhaps this is what is causing the issue: https://github.com/imi-bigpicture/wsidicom/blob/fc2149e61958dd6d69baeae02a7da3fef8969533/wsidicom/series.py#L407-L436, called from read_region

measty · 2023-03-17T02:06:24Z

I can't find where in the wsidicom source or documentation this behaviour is specified. Can you provide any reference? It's not really a DICOM thing but a wsidicom thing IMO.

Yes, its a wsidicom thing. it's not specifically documented, but you can see it in the source, for example:

    def calculate_scale(self, level_to: int) -> int:
        """Return scaling factor to given level.
        Parameters
        ----------
        level_to -- index of level to scale to
        Returns
        ----------
        int
            Scaling factor between this level and given level
        """
        return int(2 ** (level_to - self.level))

    def _assign_level(self, base_pixel_spacing: SizeMm) -> int:
        """Return (2^level scale factor) based on pixel spacing.
        Will round to closest integer. Raises NotImplementedError if level is
        to far from integer.
        Parameters
        ----------
        base_pixel_spacing: SizeMm
            The pixel spacing of the base lavel
        Returns
        ----------
        int
            The pyramid order of the level
        """
        float_level = math.log2(
            self.pixel_spacing.width/base_pixel_spacing.width
        )
        level = int(round(float_level))
        TOLERANCE = 1e-2
        if not math.isclose(float_level, level, rel_tol=TOLERANCE):
            raise NotImplementedError("Levels needs to be integer.")
        return level

from which you can see that the level is defined as the power of 2 between the pixel size at that level compared to base resolution, as opposed to being an index into a list of levels with the scale information stored independently from that as in tiatoolbox WSIReader.

So what wsidicom is expecting as the level in read_region is log2(level_downsamples[read_level]) , not read_level. (or we can just get the level directly from the wsidicom level instance, as in the PR here)

John-P · 2023-03-17T11:52:42Z

@measty thanks for finding that! Just wanted to verify the behaviour before we change it. Looks good.

shaneahmed

Thanks @measty

@navidstuv

## 1.4.0 (2023-04-24) ### Major Updates and Feature Improvements - Adds Python 3.11 support \[experimental\] #500 - Python 3.11 is not fully supported by `pytorch` pytorch/pytorch#86566 and `openslide` openslide/openslide-python#188 - Removes Python 3.7 support - This allows upgrading all the dependencies which were dependent on an older version of Python. - Adds Neighbourhood Querying Support To AnnotationStore #540 - This enables easy and efficient querying of annotations within a neighbourhood of other annotations. - Adds `MultiTaskSegmentor` engine #424 - Fixes an issue with stain augmentation to apply augmentation to only tissue regions. - #546 contributed by @navidstuv - Filters logger output to stdout instead of stderr. - Fixes #255 - Allows import of some modules at higher level for improved usability - `WSIReader` can now be imported as `from tiatoolbox.wsicore import WSIReader` - `WSIMeta` can now be imported as `from tiatoolbox.wsicore import WSIMeta` - `HoVerNet`, `HoVerNetPlus`, `IDaRS`, `MapDe`, `MicroNet`, `NuClick`, `SCCNN` can now be imported as \`from tiatoolbox.models import HoVerNet, HoVerNetPlus, IDaRS, MapDe, MicroNet, NuClick, SCCNN - Improves `PatchExtractor` performance. Updates `WSIPatchDataset` to be consistent. #571 - Updates documentation for `License` for clarity on source code and model weights license. ### Changes to API - Updates SCCNN architecture to make it consistent with other models. #544 ### Bug Fixes and Other Changes - Fixes Parsing Missing Omero Version NGFF Metadata #568 - Fixes #535 raised by @benkamphaus - Fixes reading of DICOM WSIs at the correct level #564 - Fixes #529 - Fixes `scipy`, `matplotlib`, `scikit-image` deprecated code - Fixes breaking changes in `DICOMWSIReader` to make it compatible with latest `wsidicom` version. #539, #580 - Updates `shapely` dependency to version >=2.0.0 and fixes any breaking changes. - Fixes bug with `DictionaryStore.bquery` and `geometry=None`, i.e. only a where predicate given. - Partly Fixes #532 raised by @blaginin - Fixes local tests for Windows/Linux - Fixes `flake8`, `deepsource` errors. - Uses `logger` instead of `warnings` and `print` statements to properly log runs. ### Development related changes - Upgrades dependencies which are dependent on Python 3.7 - Moves `requirements*.txt` files to `requirements` folder - Removes `tox` - Uses `pyproject.toml` for `bdist_wheel`, `pytest` and `isort` - Adds `joblib` and `numba` as dependencies.

@navidstuv

## 1.4.0 (2023-04-24) ### Major Updates and Feature Improvements - Adds Python 3.11 support \[experimental\] #500 - Python 3.11 is not fully supported by `pytorch` pytorch/pytorch#86566 and `openslide` openslide/openslide-python#188 - Removes Python 3.7 support - This allows upgrading all the dependencies which were dependent on an older version of Python. - Adds Neighbourhood Querying Support To AnnotationStore #540 - This enables easy and efficient querying of annotations within a neighbourhood of other annotations. - Adds `MultiTaskSegmentor` engine #424 - Fixes an issue with stain augmentation to apply augmentation to only tissue regions. - #546 contributed by @navidstuv - Filters logger output to stdout instead of stderr. - Fixes #255 - Allows import of some modules at higher level for improved usability - `WSIReader` can now be imported as `from tiatoolbox.wsicore import WSIReader` - `WSIMeta` can now be imported as `from tiatoolbox.wsicore import WSIMeta` - `HoVerNet`, `HoVerNetPlus`, `IDaRS`, `MapDe`, `MicroNet`, `NuClick`, `SCCNN` can now be imported as \`from tiatoolbox.models import HoVerNet, HoVerNetPlus, IDaRS, MapDe, MicroNet, NuClick, SCCNN - Improves `PatchExtractor` performance. Updates `WSIPatchDataset` to be consistent. #571 - Updates documentation for `License` for clarity on source code and model weights license. ### Changes to API - Updates SCCNN architecture to make it consistent with other models. #544 ### Bug Fixes and Other Changes - Fixes Parsing Missing Omero Version NGFF Metadata #568 - Fixes #535 raised by @benkamphaus - Fixes reading of DICOM WSIs at the correct level #564 - Fixes #529 - Fixes `scipy`, `matplotlib`, `scikit-image` deprecated code - Fixes breaking changes in `DICOMWSIReader` to make it compatible with latest `wsidicom` version. #539, #580 - Updates `shapely` dependency to version >=2.0.0 and fixes any breaking changes. - Fixes bug with `DictionaryStore.bquery` and `geometry=None`, i.e. only a where predicate given. - Partly Fixes #532 raised by @blaginin - Fixes local tests for Windows/Linux - Fixes `flake8`, `deepsource` errors. - Uses `logger` instead of `warnings` and `print` statements to properly log runs. ### Development related changes - Upgrades dependencies which are dependent on Python 3.7 - Moves `requirements*.txt` files to `requirements` folder - Removes `tox` - Uses `pyproject.toml` for `bdist_wheel`, `pytest` and `isort` - Adds `joblib` and `numba` as dependencies.

fix level when using dicom read_region

4d6fe9e

measty mentioned this pull request Mar 10, 2023

slide_thumbnail method does not work properly with DICOM WSIs #529

Closed

measty requested a review from John-P March 10, 2023 19:22

measty changed the title ~~fix level when using dicom read_region~~ BUG: fix level when using dicom read_region Mar 10, 2023

shaneahmed changed the title ~~BUG: fix level when using dicom read_region~~ 🐛 BUG: fix level when using dicom read_region Mar 10, 2023

shaneahmed changed the title ~~🐛 BUG: fix level when using dicom read_region~~ 🐛 BUG: Fix Level When Using DICOM read_region Mar 10, 2023

shaneahmed linked an issue Mar 11, 2023 that may be closed by this pull request

slide_thumbnail method does not work properly with DICOM WSIs #529

Closed

shaneahmed assigned measty Mar 16, 2023

Merge branch 'develop' into fix-dicom-levels

b2afb5b

shaneahmed added the bug Something isn't working label Mar 17, 2023

shaneahmed approved these changes Mar 17, 2023

View reviewed changes

shaneahmed merged commit 6fb0e31 into develop Mar 17, 2023

shaneahmed deleted the fix-dicom-levels branch March 17, 2023 15:47

shaneahmed added this to the Release v1.4.0 milestone Apr 10, 2023

This was referenced May 5, 2023

🔖 Release 1.4.0 #599

Merged

🔖 Release 1.4.0 #600

Merged

Navigation Menu

Repository navigation

🐛 BUG: Fix Level When Using DICOM `read_region` - #564

🐛 BUG: Fix Level When Using DICOM `read_region`#564
shaneahmed merged 2 commits into
developfrom
fix-dicom-levels

measty commented Mar 10, 2023 •

edited

Loading

Uh oh!

codecov Bot commented Mar 10, 2023 •

edited

Loading

Uh oh!

measty commented Mar 16, 2023

Uh oh!

John-P commented Mar 17, 2023 •

edited

Loading

Uh oh!

measty commented Mar 17, 2023

Uh oh!

John-P commented Mar 17, 2023

Uh oh!

shaneahmed left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

measty commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

measty commented Mar 16, 2023

Uh oh!

John-P commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

measty commented Mar 17, 2023

Uh oh!

John-P commented Mar 17, 2023

Uh oh!

shaneahmed left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

measty commented Mar 10, 2023 •

edited

Loading

codecov Bot commented Mar 10, 2023 •

edited

Loading

John-P commented Mar 17, 2023 •

edited

Loading