Skip to content

Fix redundancy in test WSI image #5878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bhashemian wants to merge 7 commits into from
Closed

Fix redundancy in test WSI image #5878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c382483
Fix redundant extension .tiff.tiff and basename
bhashemian Jan 19, 2023
e19ae5d
Merge branch 'dev' into fix-extensions
bhashemian Jan 20, 2023
488ac6b
Merge branch 'fix-extensions' of github.com:drbeh/MONAI; branch 'dev'…
bhashemian Jan 20, 2023
317a58e
update to wsi_img.tiff
bhashemian Jan 20, 2023
3d86b73
Merge branch 'dev' into fix-extensions
bhashemian Jan 23, 2023
22c6883
Merge branch 'dev' into fix-extensions
bhashemian Jan 23, 2023
da6d909
Merge branch 'dev' into fix-extensions
bhashemian Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions tests/test_masked_inference_wsi_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,10 @@
_, has_cim = optional_import("cucim", name="CuImage")
_, has_osl = optional_import("openslide")

FILE_KEY = "wsi_img"
FILE_KEY = "wsi_img.tiff"
FILE_URL = testing_data_config("images", FILE_KEY, "url")
base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
Copy link
Contributor

@wyli wyli Jan 20, 2023

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@bhashemian bhashemian Jan 20, 2023

Choose a reason for hiding this comment

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

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

FILE_NAME = f"temp_{FILE_KEY}.tiff"

Copy link
Member Author

@bhashemian bhashemian Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@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.

Copy link
Member Author

@bhashemian bhashemian Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@bhashemian bhashemian Jan 23, 2023

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@wyli wyli Jan 23, 2023

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

@bhashemian bhashemian Jan 23, 2023

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

@wyli wyli Jan 23, 2023

Choose a reason for hiding this comment

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

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)?

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

MASK1, MASK2, MASK4 = "mask1.npy", "mask2.npy", "mask4.npy"

Expand Down
5 changes: 2 additions & 3 deletions tests/test_masked_patch_wsi_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@
_, has_codec = optional_import("imagecodecs")
has_tiff = has_tiff and has_codec

FILE_KEY = "wsi_img"
FILE_KEY = "wsi_img.tiff"
FILE_URL = testing_data_config("images", FILE_KEY, "url")
base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", "temp_" + base_name + extension)
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", f"temp_{FILE_KEY}")

TEST_CASE_0 = [
{"data": [{"image": FILE_PATH, WSIPatchKeys.LEVEL: 8, WSIPatchKeys.SIZE: (2, 2)}], "mask_level": 8},
Expand Down
5 changes: 2 additions & 3 deletions tests/test_patch_wsi_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@
_, has_codec = optional_import("imagecodecs")
has_tiff = has_tiff and has_codec

FILE_KEY = "wsi_img"
FILE_KEY = "wsi_img.tiff"
FILE_URL = testing_data_config("images", FILE_KEY, "url")
base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", "temp_" + base_name + extension)
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", f"temp_{FILE_KEY}")

TEST_CASE_DEP_0 = [
{
Expand Down
5 changes: 2 additions & 3 deletions tests/test_sliding_patch_wsi_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@
_, has_codec = optional_import("imagecodecs")
has_tiff = has_tiff and has_codec

FILE_KEY = "wsi_img"
FILE_KEY = "wsi_img.tiff"
FILE_URL = testing_data_config("images", FILE_KEY, "url")
base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", "temp_" + base_name + extension)
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", f"temp_{FILE_KEY}")

FILE_PATH_SMALL_0 = os.path.join(os.path.dirname(__file__), "testing_data", "temp_wsi_inference_0.tiff")
FILE_PATH_SMALL_1 = os.path.join(os.path.dirname(__file__), "testing_data", "temp_wsi_inference_1.tiff")
Expand Down
5 changes: 2 additions & 3 deletions tests/test_smartcache_patch_wsi_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@
_cucim, has_cim = optional_import("cucim")
has_cim = has_cim and hasattr(_cucim, "CuImage")

FILE_KEY = "wsi_img"
FILE_KEY = "wsi_img.tiff"
FILE_URL = testing_data_config("images", FILE_KEY, "url")
base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", "temp_" + base_name + extension)
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", f"temp_{FILE_KEY}")

TEST_CASE_0 = [
{
Expand Down
6 changes: 2 additions & 4 deletions tests/test_wsireader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@
_, has_codec = optional_import("imagecodecs")
has_tiff = has_tiff and has_codec

FILE_KEY = "wsi_img"
FILE_KEY = "wsi_img.tiff"
FILE_URL = testing_data_config("images", FILE_KEY, "url")
base_name, extension = os.path.basename(f"{FILE_URL}"), ".tiff"
FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", "temp_" + base_name + extension)

FILE_PATH = os.path.join(os.path.dirname(__file__), "testing_data", f"temp_{FILE_KEY}")
HEIGHT = 32914
WIDTH = 46000

Expand Down
2 changes: 1 addition & 1 deletion tests/testing_data/data_config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"images": {
"wsi_img": {
"wsi_img.tiff": {
"url": "https://github.com/Project-MONAI/MONAI-extra-test-data/releases/download/0.8.1/CMU-1.tiff",
"hash_type": "sha256",
"hash_val": "73a7e89bc15576587c3d68e55d9bf92f09690280166240b48ff4b48230b13bcd"
Expand Down