Skip to content

Conversation

@bhashemian
Copy link
Member

Fixes #3256

Status

Ready

Types of changes

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

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian requested review from Nic-Ma and wyli November 4, 2021 03:12
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 4, 2021

/build

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 4, 2021

Hi @drbeh ,

Is there any unit test to cover this spatial_shape property?

Thanks.

@bhashemian
Copy link
Member Author

Hi @drbeh ,

Is there any unit test to cover this spatial_shape property?

Thanks.

@Nic-Ma, it is not part of the functionality of WSIReader and I am not sure how it is being used in the rest of MONAI. Do we have any documentation on this? so that I know what type of unit test it requires. Thanks

@bhashemian bhashemian merged commit 6944ac0 into Project-MONAI:dev Nov 4, 2021
@bhashemian bhashemian deleted the fix-3256 branch November 4, 2021 14:42
@wyli
Copy link
Contributor

wyli commented Nov 4, 2021

Hi @drbeh ,
Is there any unit test to cover this spatial_shape property?
Thanks.

@Nic-Ma, it is not part of the functionality of WSIReader and I am not sure how it is being used in the rest of MONAI. Do we have any documentation on this? so that I know what type of unit test it requires. Thanks

If we don't know how it is being used and how to verify this feature, why do we create and merge this PR?

@bhashemian
Copy link
Member Author

bhashemian commented Nov 4, 2021

You're right @wyli! But it was an obvious bug to fix. I don't know how and why you are using it but I only know an instance that it failed. #3256
I will make a unit test out of it but it's better to have a concrete unit test for all elements of these dictionaries that are being passed.

@bhashemian
Copy link
Member Author

@wyli @Nic-Ma, I added a new test for WSIReader that covers the fixed issue here. https://github.com/Project-MONAI/MONAI/pull/3261/files

@bhashemian bhashemian mentioned this pull request Nov 5, 2021
4 tasks
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.

None value in image_meta_dict of WSIReader

3 participants