Fix redundancy in test WSI image

[bhashemian]

Description

The test WSI image is downloaded to a file with redundant extension .tiff.tiff and also the code has unused variables defined like basename. This PR fix that and simplify the WSI name.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).

[Nic-Ma]

Hi @drbeh ,

May I know which line in the original code causes "redundant extension .tiff.tiff" as you described?
I didn't find it immediately.

Thanks in advance.

[bhashemian]

Hi @drbeh ,

May I know which line in the original code causes "redundant extension .tiff.tiff" as you described? I didn't find it immediately.

Thanks in advance.

Hi @Nic-Ma, this is the line that generates extra extension:

base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", FILE_NAME + extension)

os.path.basename return the base part of the URL and not the base part of filename. To separate extension, one should use os.path.splitext. Since the basename already have .tiff, the second line add another extension to it.

base_name, extension = os.path.splitext(os.path.basename(f"{FILE_URL}"))

But it is not needed here as we don't need to extract the extension on the first place.

[wyli]

to me the filename ending with .tiff.tiff is somewhat a better test case than a regular image_name.tiff, showing that the reader's file extension matching works fine

[bhashemian]

I might be missing something, but in this new way also the extension is explicitly set:

FILE_NAME = f"temp_{FILE_KEY}.tiff"

[bhashemian]

@wyli reading your comment again, I realized what you mean and it's a good point. I think a better way to do it is to explicitly mentioning it in the FILE_KEY (i.e. wsi_img.tiff instead of wsi_img) and not adding any additional extension. In this way, the developer who is writing the test knows that it is using a tiff file. Otherwise, it is not visible to the developer what the actual format of the file is unless check the file that is created in the testing_data.

I will update the PR to address your point.

[bhashemian]

Updated the PR. Let me know if you have any comment. Thanks

[bhashemian]

@wyli please let me know if you have any other comment. Thanks

[wyli]

thanks for the updates, I checked out the code and ran the test cases, still I don't fully understand why these code changes are needed. were you seeing some unexpected outcomes of the code?

[bhashemian]

@wyli I believe this PR now makes improvement to two concerns:

1. the developer who knows what image is being used (CMU-1.tiff from Carnegie Melon University) doesn't expect to deal with .tiff.tiff (the original goal of this PR).
2. the users that just use the testing_data_config API to write unittests don't know what's the format of wsi_img is. Also WSI images are in multiple format (.tiff, .svs, .mrxs, etc.) so making it explicit about wsi_image.tiff would be helpful to avoid confusions.

[wyli]

thanks, I think in general it's good to simulate some corner cases as the user's input to detect potential issues. if multiple format support is important, shall we focus on generating test cases in different formats (and may be combined with different variants of file extension names)?

[bhashemian]

closing in favor of #6244.