Skip to content

Conversation

@wyli
Copy link
Contributor

@wyli wyli commented Jul 16, 2021

fixes #2613
fixes #1629
fixes #1357
fixes #1713 (by upgrading itk)
fixes #1711

Description

  • using the numpy array layout for the get_data of itkreader,pilreader, so that the loader results are consistent
  • itkreader's affine is converted to nibabel's convention to be consistent
  • python -m tests.test_distributed_sampler still gives messages such as UserWarning: semaphore_tracker: There appear to be 1 leaked semaphores to clean up at shutdown because of itk 5.2.0 semaphore leaks addressed by workaround ac7944e

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.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@wyli wyli force-pushed the 1629-update-itk-dep branch 3 times, most recently from 4a86ac6 to 24c71e1 Compare July 16, 2021 09:01
@wyli
Copy link
Contributor Author

wyli commented Jul 16, 2021

/integration-test

/black

@wyli wyli force-pushed the 1629-update-itk-dep branch from 24c71e1 to 4a86ac6 Compare July 16, 2021 09:33
@wyli wyli marked this pull request as ready for review July 16, 2021 14:54
@wyli wyli changed the title [wip] 2613 saver/loader consistencies 2613 saver/loader consistencies Jul 16, 2021
@madil90
Copy link
Contributor

madil90 commented Jul 19, 2021

/build

@wyli wyli requested a review from ericspod July 19, 2021 22:53
@wyli wyli force-pushed the 1629-update-itk-dep branch from 04b2876 to ce62b54 Compare July 21, 2021 11:47
wyli added 13 commits July 21, 2021 09:18
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli force-pushed the 1629-update-itk-dep branch from ce62b54 to 7835727 Compare July 21, 2021 13:23
@wyli
Copy link
Contributor Author

wyli commented Jul 21, 2021

I run a few tutorials/demo with this PR and all work fine. Additional integration tests show that loadimaged with the 'itkreader' or 'nibabelreader' option gives the same results.

if idx == 1:
_readers = ("itkreader", "itkreader")
elif idx == 2:
_readers = ("itkreader", "nibabelreader")

I think this gives us some confidence in the code. Could you please review @ericspod @rijobro @Nic-Ma? The current itkreader in the dev branch is broken #1711, which means we don't have any basic working loader for DICOM.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Jul 21, 2021

Hi @wyli ,

Thanks for your quick update and test.
I just came back from a short vacation last night and still checking the ITK APIs related to the data loading and thinking about how to develop the saving feature for other formats, will review this PR ASAP.

Thanks.

Copy link
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

Looks good to except for several minor comments.

Thanks.

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli force-pushed the 1629-update-itk-dep branch from 167a933 to 116eaa5 Compare July 21, 2021 18:52
@wyli wyli enabled auto-merge (squash) July 21, 2021 19:16
@wyli wyli disabled auto-merge July 21, 2021 21:50
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli enabled auto-merge (squash) July 22, 2021 00:31
@wyli wyli merged commit 027947b into Project-MONAI:dev Jul 22, 2021
@wyli wyli deleted the 1629-update-itk-dep branch July 22, 2021 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants