🚸 Add Support for ignore_is_tiled_tiff in WSIReader.open()

[GeorgeBatch]

A possible solution for the issue #804

Checks if openslide is able to detect tiff format. In this case, OpenSlide opens the tiff file, otherwise falls back to tifffile or opencv.

[shaneahmed]

Thanks @GeorgeBatch. That's great. If you can provide us with a sample tif then we can upload this to our servers and set up automated tests. We can also investigate if there is another work around otherwise we can add tests and accept this PR.

Please can you provide a onedrive/dropbox/gdrive link to sample tif file and send it to tialab@dcs.warwick.ac.uk so we can upload to tiatest server. Please make sure that the test file is shared under BSD-3 license or an open license otherwise we will not be able to upload the file.

If you can find a file which is small in size that would be useful as it would mean tests run fast on GitHub actions.

[GeorgeBatch]

@shaneahmed, I will try to get you access to one of our images as soon as possible. They come from an ongoing study so I might need to request a small dummy image scanned with the same scanner.

[shaneahmed]

@shaneahmed, I will try to get you access to one of our images as soon as possible. They come from an ongoing study so I might need to request a small dummy image scanned with the same scanner.

A dummy slide would be great.

[shaneahmed]

Updated branch with develop to fix integration with codecov.

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 99.87%. Comparing base (ec5a373) to head (5b488c7).

Files	Patch %	Lines
tiatoolbox/wsicore/wsireader.py	75.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #807      +/-   ##
===========================================
- Coverage    99.89%   99.87%   -0.03%     
===========================================
  Files           69       69              
  Lines         8574     8577       +3     
  Branches      1640     1642       +2     
===========================================
+ Hits          8565     8566       +1     
  Misses           1        1              
- Partials         8       10       +2     

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

[GeorgeBatch]

@shaneahmed, I will share a dummy slide with you as soon as I have it. Hopefully, this week.

[GeorgeBatch]

@shaneahmed, I shared some samples through OneDrive with tialab@dcs.warwick.ac.uk. I made slides myself using flower petals so they can be shared without restrictions.

[GeorgeBatch]

Should the argument ignore_is_tiled_tiff be also part of the __init__() method of the WSIReader class?

[shaneahmed]

Should the argument ignore_is_tiled_tiff be also part of the __init__() method of the WSIReader class?

The best way would be automatically identify metadata from the file and then read. If that's not feasible then we can consider adding ignore_is_tiled_tiff as keyword argument. I have requested access to the files by email.

[shaneahmed]

The test file is uploaded to https://tiatoolbox.dcs.warwick.ac.uk/sample_wsis/ventana_sample.tif

You can update https://github.com/TissueImageAnalytics/tiatoolbox/blob/develop/tiatoolbox/data/remote_samples.yaml to add this for tests.

[GeorgeBatch]

@shaneahmed I added the file to the registry. I do not think Ventana scanners always output .tif files. I think it was just set up like that for us. So maybe it would be good to name it sample-ventana-tif

I have 2 questions about this code

tiatoolbox/tiatoolbox/wsicore/wsireader.py

Lines 300 to 307 in ec5a373

	if suffixes[-2:] in ([".ome", ".tiff"],):
	return TIFFWSIReader(input_path, mpp=mpp, power=power)
	if last_suffix in (".tif", ".tiff") and is_tiled_tiff(input_path):
	try:
	return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
	except openslide.OpenSlideError:
	return TIFFWSIReader(input_path, mpp=mpp, power=power)

1. What's the reason for not including .tif as a suffix to be processed with TIFFWSIReader on line 300?
2. Why is OpenSlideWSIReader used as default instead of TIFFWSIReader, which is only called on if OpenSlideWSIReader fails?

I am unsure which of the cases to use if the flag ignore_is_tiled_tiff is not desirable.

“vendor” field in info is empty, so can’t use it for deciding which backend to use for WSIReader.

[GeorgeBatch]

I tried adding the new slide to the fixture list with OpenSlideWSIreader as the expected reader:

        ...
        {
            "reader_class": OpenSlideWSIReader,
            "sample_key": "ventana-tif",
            "kwargs": {},
        },
        ...
    ids=[
        ...
        "OpenSlideReader (Ventana non-tiled tif)",
        ...

tiatoolbox/tests/test_wsireader.py

Lines 2512 to 2573 in ec5a373

	@pytest.fixture(
	scope="module",
	params=[
	{
	"reader_class": AnnotationStoreReader,
	"sample_key": "annotation_store_svs_1",
	"kwargs": {
	"base_wsi_key": "svs-1-small",
	"renderer": AnnotationRenderer(
	"type",
	COLOR_DICT,
	),
	"alpha": 0.5,
	},
	},
	{
	"reader_class": AnnotationStoreReader,
	"sample_key": "annotation_store_svs_1",
	"kwargs": {
	"renderer": AnnotationRenderer(
	"type",
	COLOR_DICT,
	blur_radius=3,
	),
	},
	},
	{
	"reader_class": TIFFWSIReader,
	"sample_key": "ome-brightfield-pyramid-1-small",
	"kwargs": {},
	},
	{
	"reader_class": DICOMWSIReader,
	"sample_key": "dicom-1",
	"kwargs": {},
	},
	{
	"reader_class": NGFFWSIReader,
	"sample_key": "ngff-1",
	"kwargs": {},
	},
	{
	"reader_class": OpenSlideWSIReader,
	"sample_key": "svs-1-small",
	"kwargs": {},
	},
	{
	"reader_class": JP2WSIReader,
	"sample_key": "jp2-omnyx-small",
	"kwargs": {},
	},
	],
	ids=[
	"AnnotationReaderOverlaid",
	"AnnotationReaderMaskOnly",
	"TIFFReader",
	"DICOMReader",
	"NGFFWSIReader",
	"OpenSlideWSIReader (Small SVS)",
	"OmnyxJP2WSIReader",
	],
	)

But the test_base_open() test fails with

======================================================================================================================================== FAILURES ========================================================================================================================================
________________________________________________________________________________________________________________ test_base_open[OpenSlideReader (Ventana non-tiled tif)] _________________________________________________________________________________________________________________

wsi = <tiatoolbox.wsicore.wsireader.OpenSlideWSIReader object at 0x16e0c7a90>

    def test_base_open(wsi: WSIReader) -> None:
        """Checks that WSIReader.open detects the type correctly."""
        new_wsi = WSIReader.open(wsi.input_path)
>       assert type(new_wsi) is type(wsi)
E       AssertionError: assert <class 'tiatoolbox.wsicore.wsireader.VirtualWSIReader'> is <class 'tiatoolbox.wsicore.wsireader.OpenSlideWSIReader'>
E        +  where <class 'tiatoolbox.wsicore.wsireader.VirtualWSIReader'> = type(<tiatoolbox.wsicore.wsireader.VirtualWSIReader object at 0x3189f3050>)
E        +  and   <class 'tiatoolbox.wsicore.wsireader.OpenSlideWSIReader'> = type(<tiatoolbox.wsicore.wsireader.OpenSlideWSIReader object at 0x16e0c7a90>)

tests/test_wsireader.py:2611: AssertionError

because, unlike in test_wsireader_open(), I do not know how to pass the ignore_is_tiled_tiff argument within the pytest fixture (and I am not sure it should be passed), so the is_tiled_tiff check is performed, fails, and VirtualWSIReader is used.

    wsi = WSIReader.open(sample_ventana_tif, ignore_is_tiled_tiff=True)
    assert isinstance(wsi, wsireader.OpenSlideWSIReader)

[shaneahmed]

@shaneahmed I added the file to the registry. I do not think Ventana scanners always output .tif files. I think it was just set up like that for us. So maybe it would be good to name it sample-ventana-tif

I have 2 questions about this code

tiatoolbox/tiatoolbox/wsicore/wsireader.py

Lines 300 to 307 in ec5a373

	if suffixes[-2:] in ([".ome", ".tiff"],):
	return TIFFWSIReader(input_path, mpp=mpp, power=power)
	if last_suffix in (".tif", ".tiff") and is_tiled_tiff(input_path):
	try:
	return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
	except openslide.OpenSlideError:
	return TIFFWSIReader(input_path, mpp=mpp, power=power)

1. What's the reason for not including .tif as a suffix to be processed with TIFFWSIReader on line 300?
2. Why is OpenSlideWSIReader used as default instead of TIFFWSIReader, which is only called on if OpenSlideWSIReader fails?

I am unsure which of the cases to use if the flag ignore_is_tiled_tiff is not desirable.

“vendor” field in info is empty, so can’t use it for deciding which backend to use for WSIReader.

The answer to both your questions is the same. When we wrote the code tifffile had not good support for WSIs. It still has very limited documentation. OpenSlide is preferred library to read WSIs by researchers. So most researchers will find it easy to port their code to tiatoolbox and integrate into their pipelines. If OpenSlide fails then the fallback option is tifffile.

[shaneahmed]

I tried adding the new slide to the fixture list with OpenSlideWSIreader as the expected reader:

        ...
        {
            "reader_class": OpenSlideWSIReader,
            "sample_key": "ventana-tif",
            "kwargs": {},
        },
        ...
    ids=[
        ...
        "OpenSlideReader (Ventana non-tiled tif)",
        ...

tiatoolbox/tests/test_wsireader.py

Lines 2512 to 2573 in ec5a373

	@pytest.fixture(
	scope="module",
	params=[
	{
	"reader_class": AnnotationStoreReader,
	"sample_key": "annotation_store_svs_1",
	"kwargs": {
	"base_wsi_key": "svs-1-small",
	"renderer": AnnotationRenderer(
	"type",
	COLOR_DICT,
	),
	"alpha": 0.5,
	},
	},
	{
	"reader_class": AnnotationStoreReader,
	"sample_key": "annotation_store_svs_1",
	"kwargs": {
	"renderer": AnnotationRenderer(
	"type",
	COLOR_DICT,
	blur_radius=3,
	),
	},
	},
	{
	"reader_class": TIFFWSIReader,
	"sample_key": "ome-brightfield-pyramid-1-small",
	"kwargs": {},
	},
	{
	"reader_class": DICOMWSIReader,
	"sample_key": "dicom-1",
	"kwargs": {},
	},
	{
	"reader_class": NGFFWSIReader,
	"sample_key": "ngff-1",
	"kwargs": {},
	},
	{
	"reader_class": OpenSlideWSIReader,
	"sample_key": "svs-1-small",
	"kwargs": {},
	},
	{
	"reader_class": JP2WSIReader,
	"sample_key": "jp2-omnyx-small",
	"kwargs": {},
	},
	],
	ids=[
	"AnnotationReaderOverlaid",
	"AnnotationReaderMaskOnly",
	"TIFFReader",
	"DICOMReader",
	"NGFFWSIReader",
	"OpenSlideWSIReader (Small SVS)",
	"OmnyxJP2WSIReader",
	],
	)

But the test_base_open() test fails with

======================================================================================================================================== FAILURES ========================================================================================================================================
________________________________________________________________________________________________________________ test_base_open[OpenSlideReader (Ventana non-tiled tif)] _________________________________________________________________________________________________________________

wsi = <tiatoolbox.wsicore.wsireader.OpenSlideWSIReader object at 0x16e0c7a90>

    def test_base_open(wsi: WSIReader) -> None:
        """Checks that WSIReader.open detects the type correctly."""
        new_wsi = WSIReader.open(wsi.input_path)
>       assert type(new_wsi) is type(wsi)
E       AssertionError: assert <class 'tiatoolbox.wsicore.wsireader.VirtualWSIReader'> is <class 'tiatoolbox.wsicore.wsireader.OpenSlideWSIReader'>
E        +  where <class 'tiatoolbox.wsicore.wsireader.VirtualWSIReader'> = type(<tiatoolbox.wsicore.wsireader.VirtualWSIReader object at 0x3189f3050>)
E        +  and   <class 'tiatoolbox.wsicore.wsireader.OpenSlideWSIReader'> = type(<tiatoolbox.wsicore.wsireader.OpenSlideWSIReader object at 0x16e0c7a90>)

tests/test_wsireader.py:2611: AssertionError

because, unlike in test_wsireader_open(), I do not know how to pass the ignore_is_tiled_tiff argument within the pytest fixture (and I am not sure it should be passed), so the is_tiled_tiff check is performed, fails, and VirtualWSIReader is used.

    wsi = WSIReader.open(sample_ventana_tif, ignore_is_tiled_tiff=True)
    assert isinstance(wsi, wsireader.OpenSlideWSIReader)

After a bit of play around with your image. I think I have a more minimalistic solution which will not change the API. If you make the following change in the code, then we do not need a flag ignore_is_tiled_tiff.

If you change the line 309 with this code, tiatoolbox should work on ventanna scanners.

        if last_suffix in (".tif", ".tiff"):
            if openslide.OpenSlide.detect_format(input_path) is not None:
                return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

If you still want to force TIFFWSIReader then there are two ways:

1. Use TIFFWSIReader() to open the image.
2. We change the API to allow specifying which class to open the file. But it would be redundant considering we can open an image directly by specifying a class.

Some tests may fail after this change but we can fix those easily, I think.

[GeorgeBatch]

Just replacing the old code with your proposed code results in the following error:

========================================================================================== FAILURES ===========================================================================================
__________________________________________________________________________________ test_tiled_tiff_tifffile ___________________________________________________________________________________

remote_sample = <function remote_sample.<locals>.__remote_sample at 0x30d98fce0>

    def test_tiled_tiff_tifffile(remote_sample: Callable) -> None:
        """Test fallback to tifffile for files which openslide cannot read.
    
        E.G. tiled tiffs with JPEG XL compression.
    
        """
        sample_path = remote_sample("tiled-tiff-1-small-jp2k")
>       wsi = wsireader.WSIReader.open(sample_path)

tests/test_wsireader.py:2005: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tiatoolbox/wsicore/wsireader.py:305: in open
    return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
tiatoolbox/wsicore/wsireader.py:1663: in __init__
    self.openslide_wsi = openslide.OpenSlide(filename=str(self.input_path))
/Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/openslide/__init__.py:179: in __init__
    self._osr = lowlevel.open(str(filename))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

result = 5811949952, _func = <_FuncPtr object at 0x30c3041d0>
_args = ('/private/var/folders/lz/602r_tbj50jf1yw4jnmxbg9r0000gn/T/pytest-of-gbatch/pytest-17/data0/CMU-1-Small-Region.jp2k.tiff',)

    def _check_open(result, _func, _args):
        if result is None:
            raise OpenSlideUnsupportedFormatError("Unsupported or missing image file")
        slide = _OpenSlide(c_void_p(result))
        err = get_error(slide)
        if err is not None:
>           raise OpenSlideError(err)
E           openslide.lowlevel.OpenSlideError: Unsupported TIFF compression: 33004

/Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/openslide/lowlevel.py:207: OpenSlideError
====================================================================================== warnings summary =======================================================================================
../../../../../opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/_pytest/config/__init__.py:1439
  /Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/_pytest/config/__init__.py:1439: PytestConfigWarning: Unknown config option: collect_ignore
  
    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

tests/test_wsireader.py::test_convert_resolution_units
  /Users/gbatch/Developer/projects/current/comp-path/tiatoolbox/tiatoolbox/wsicore/wsireader.py:888: RuntimeWarning: divide by zero encountered in scalar divide
    output_dict["baseline"] = baseline_mpp[0] / output_dict["mpp"][0]

tests/test_wsireader.py::test_tissue_mask_otsu
  /Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/defusedxml/__init__.py:30: DeprecationWarning: defusedxml.cElementTree is deprecated, import from defusedxml.ElementTree instead.
    from . import cElementTree

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================================== short test summary info ===================================================================================
FAILED tests/test_wsireader.py::test_tiled_tiff_tifffile - openslide.lowlevel.OpenSlideError: Unsupported TIFF compression: 33004

The image 'CMU-1-Small-Region.jp2k.tiff' passes the openslide.OpenSlide.detect_format(input_path) check, but openslide fails to open it.

            if openslide.OpenSlide.detect_format(input_path) is not None:
                return OpenSlideWSIReader(input_path, mpp=mpp, power=power)

---

To mitigate this, a try-except block can be used:

        if last_suffix in (".tif", ".tiff"):
            if openslide.OpenSlide.detect_format(input_path) is not None:
                try:
                    return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
                except openslide.OpenSlideError:
                    pass
            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

To preserve as much of the original behaviour as possible, these lines need to be outside of the except block.

            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

[GeorgeBatch]

All tests pass with this change. But this change resulted in some failed ruff checks - the WSIReader.open() becomes too complex. So I used git commit --no-verify so you can see the changes.

tiatoolbox/wsicore/wsireader.py:231:9: PLR0912 Too many branches (14 > 12)
tiatoolbox/wsicore/wsireader.py:231:9: C901 `open` is too complex (15 > 14)
Found 2 errors.

Should I add PLR0912 and C901 to the noqa of open? I know it's not a good strategy to ignore linters whenever they complain.
Now:

    def open(  # noqa: PLR0911

Can be:

    def open(  # noqa: PLR0911, PLR0912, C901

[shaneahmed]

All tests pass with this change. But this change resulted in some failed ruff checks - the WSIReader.open() becomes too complex. So I used git commit --no-verify so you can see the changes.

tiatoolbox/wsicore/wsireader.py:231:9: PLR0912 Too many branches (14 > 12)
tiatoolbox/wsicore/wsireader.py:231:9: C901 `open` is too complex (15 > 14)
Found 2 errors.

Should I add PLR0912 and C901 to the noqa of open? I know it's not a good strategy to ignore linters whenever they complain. Now:

    def open(  # noqa: PLR0911

Can be:

    def open(  # noqa: PLR0911, PLR0912, C901

Please go ahead and add these. I can fix these later in a separate PR.

[shaneahmed]

Just replacing the old code with your proposed code results in the following error:

========================================================================================== FAILURES ===========================================================================================
__________________________________________________________________________________ test_tiled_tiff_tifffile ___________________________________________________________________________________

remote_sample = <function remote_sample.<locals>.__remote_sample at 0x30d98fce0>

    def test_tiled_tiff_tifffile(remote_sample: Callable) -> None:
        """Test fallback to tifffile for files which openslide cannot read.
    
        E.G. tiled tiffs with JPEG XL compression.
    
        """
        sample_path = remote_sample("tiled-tiff-1-small-jp2k")
>       wsi = wsireader.WSIReader.open(sample_path)

tests/test_wsireader.py:2005: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tiatoolbox/wsicore/wsireader.py:305: in open
    return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
tiatoolbox/wsicore/wsireader.py:1663: in __init__
    self.openslide_wsi = openslide.OpenSlide(filename=str(self.input_path))
/Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/openslide/__init__.py:179: in __init__
    self._osr = lowlevel.open(str(filename))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

result = 5811949952, _func = <_FuncPtr object at 0x30c3041d0>
_args = ('/private/var/folders/lz/602r_tbj50jf1yw4jnmxbg9r0000gn/T/pytest-of-gbatch/pytest-17/data0/CMU-1-Small-Region.jp2k.tiff',)

    def _check_open(result, _func, _args):
        if result is None:
            raise OpenSlideUnsupportedFormatError("Unsupported or missing image file")
        slide = _OpenSlide(c_void_p(result))
        err = get_error(slide)
        if err is not None:
>           raise OpenSlideError(err)
E           openslide.lowlevel.OpenSlideError: Unsupported TIFF compression: 33004

/Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/openslide/lowlevel.py:207: OpenSlideError
====================================================================================== warnings summary =======================================================================================
../../../../../opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/_pytest/config/__init__.py:1439
  /Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/_pytest/config/__init__.py:1439: PytestConfigWarning: Unknown config option: collect_ignore
  
    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

tests/test_wsireader.py::test_convert_resolution_units
  /Users/gbatch/Developer/projects/current/comp-path/tiatoolbox/tiatoolbox/wsicore/wsireader.py:888: RuntimeWarning: divide by zero encountered in scalar divide
    output_dict["baseline"] = baseline_mpp[0] / output_dict["mpp"][0]

tests/test_wsireader.py::test_tissue_mask_otsu
  /Users/gbatch/opt/miniconda3/envs/tiatoolbox-dev/lib/python3.11/site-packages/defusedxml/__init__.py:30: DeprecationWarning: defusedxml.cElementTree is deprecated, import from defusedxml.ElementTree instead.
    from . import cElementTree

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================================== short test summary info ===================================================================================
FAILED tests/test_wsireader.py::test_tiled_tiff_tifffile - openslide.lowlevel.OpenSlideError: Unsupported TIFF compression: 33004

The image 'CMU-1-Small-Region.jp2k.tiff' passes the openslide.OpenSlide.detect_format(input_path) check, but openslide fails to open it.

            if openslide.OpenSlide.detect_format(input_path) is not None:
                return OpenSlideWSIReader(input_path, mpp=mpp, power=power)

To mitigate this, a try-except block can be used:

        if last_suffix in (".tif", ".tiff"):
            if openslide.OpenSlide.detect_format(input_path) is not None:
                try:
                    return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
                except openslide.OpenSlideError:
                    pass
            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

To preserve as much of the original behaviour as possible, these lines need to be outside of the except block.

            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

I am OK with this solution. However, I think we should not pass

        if last_suffix in (".tif", ".tiff"):
            if openslide.OpenSlide.detect_format(input_path) is not None:
                try:
                    return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
                except openslide.OpenSlideError:
                    pass
            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

        if last_suffix in (".tif", ".tiff"):
            if openslide.OpenSlide.detect_format(input_path) is not None:
                try:
                    return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
                except openslide.OpenSlideError:
                    if is_tiled_tiff(input_path):
                        return TIFFWSIReader(input_path, mpp=mpp, power=power)

[GeorgeBatch]

I considered this option. My argument against was that it will make the logic different from what it was.

Before, to be opened with TIFFWSIReader, the file needed to satisfy

1. last_suffix in (".tif", ".tiff")
2. is_tiled_tiff(input_path)
3. OpenSlideWSIReader failed with openslide.OpenSlide error

This behaviour is preserved in the current option with pass.

If we put

            if is_tiled_tiff(input_path):
                return TIFFWSIReader(input_path, mpp=mpp, power=power)

into the except statement instead of the pass, to be opened with TIFFWSIReader, the file will still need to satisfy the 3 criteria as before, but in addition will need to satisfy openslide.OpenSlide.detect_format(input_path) is not None.

So, for a slide that has a non-detectable format with OpenSlide (openslide.OpenSlide.detect_format(input_path)), we will not even try to open it with TIFFWSIReader.

[GeorgeBatch]

Why is this check needed before trying OpenSlide?

            if openslide.OpenSlide.detect_format(input_path) is not None:

What do you think about the option below? This option leaves the same 3 requirements for using TIFFWSIReader as in the current version on the develop branch. Trying to open with OpenSlideWSIReader becomes easier since the file does not need to satisfy is_tiled_tiff(input_path).

        if last_suffix in (".tif", ".tiff"):
            try:
                return OpenSlideWSIReader(input_path, mpp=mpp, power=power)
            except openslide.OpenSlideError:
                if is_tiled_tiff(input_path):
                    return TIFFWSIReader(input_path, mpp=mpp, power=power)

This version passes all the existing tests.

[shaneahmed]

if openslide.OpenSlide.detect_format(input_path) is not None:

OpenSlide will still be able to open regular tiff which is not a WSI. In this case, metadata will be missing and we may get unintentional errors which are not traceable. Regular tiff file should be opened by VirtualWSIReader which can deal with regular images.

[shaneahmed]

Thanks @GeorgeBatch for your contribution. This looks good to me now.

There is an issue with test coverage which can be fulfilled by regular tiff files. I will create a separate PR for this. I will also create a separate PR for complexity issue.