Skip to content

Conversation

@bhashemian
Copy link
Member

@bhashemian bhashemian commented Nov 4, 2021

Description

This PR unifies unittests for WSIReader with different backends and also adds new tests to cover situations where WSIReader is being used in a transform and loaded via a data loader.

Related to: #3257 (comment)

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian requested review from Nic-Ma and wyli November 4, 2021 19:04
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian removed request for Nic-Ma and wyli November 4, 2021 19:45
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian changed the title Unity unittests for WSIReader Unify unittests for WSIReader Nov 4, 2021
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian requested review from Nic-Ma and wyli November 4, 2021 22:43
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian
Copy link
Member Author

@Nic-Ma @wyli , I'd appreciate your review here.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian requested a review from wyli November 5, 2021 21:14
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good to me.

@wyli
Copy link
Contributor

wyli commented Nov 6, 2021

/build

@wyli wyli enabled auto-merge (squash) November 6, 2021 09:30
@wyli wyli merged commit 7336b68 into Project-MONAI:dev Nov 6, 2021
@bhashemian bhashemian deleted the wsireader-unittests branch November 6, 2021 12:29
@wyli
Copy link
Contributor

wyli commented Nov 6, 2021

there's an error after merging this PR, any idea?

https://github.com/Project-MONAI/MONAI/actions/runs/1428856690

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 6, 2021

Hi @wyli ,

Thanks for raising the issue.
I am trying to fix it locally.

Thanks.

@wyli
Copy link
Contributor

wyli commented Nov 6, 2021

thanks @Nic-Ma , is it related to this one?
https://github.com/Project-MONAI/MONAI/runs/4125617483?check_suite_focus=true

======================================================================
ERROR: test_with_dataloader_0____w_MONAI_MONAI_tests_testing_data_temp_1sGTKZlJBIz53pfqTxoTqiIQzIoEzHLAe_tiff (tests.test_wsireader.TestCuCIM)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/conda/lib/python3.6/site-packages/parameterized/parameterized.py", line 533, in standalone_func
    return func(*(a + p.args), **p.kwargs)
  File "/__w/MONAI/MONAI/tests/test_wsireader.py", line 154, in test_with_dataloader
    spatial_shape = tuple(d.item() for d in data["image_meta_dict"]["spatial_shape"])
  File "/__w/MONAI/MONAI/tests/test_wsireader.py", line 154, in <genexpr>
    spatial_shape = tuple(d.item() for d in data["image_meta_dict"]["spatial_shape"])
ValueError: only one element tensors can be converted to Python scalars

----------------------------------------------------------------------

@Nic-Ma Nic-Ma mentioned this pull request Nov 6, 2021
7 tasks
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 6, 2021

Hi @Wenqi ,

This error message is related to my PR #3255, I think already fixed in the latest commit: d84a39b

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants