Skip to content

3595 3766 adds a base writer and an itk writer#3674

Merged
wyli merged 43 commits intoProject-MONAI:devfrom
wyli:adds-base-writer-itk-writer
Feb 7, 2022
Merged

3595 3766 adds a base writer and an itk writer#3674
wyli merged 43 commits intoProject-MONAI:devfrom
wyli:adds-base-writer-itk-writer

Conversation

@wyli
Copy link
Copy Markdown
Contributor

@wyli wyli commented Jan 18, 2022 •
edited
Loading

Signed-off-by: Wenqi Li [email protected]

part of #3595

Description

this PR adds a writer base class and an itkwriter subclass,
internally the affines are assumed in the RAS space to be aligned with nifti.

more consistency tests will be added when the nibabelwrite is in place.
the tests are ready in #3443

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • 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 --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@wyli wyli force-pushed the adds-base-writer-itk-writer branch 2 times, most recently from db5fa7f to 5c05395 Compare January 18, 2022 15:23
@wyli wyli requested review from Nic-Ma, ericspod and rijobro January 18, 2022 15:24
@ericspod ericspod mentioned this pull request Jan 18, 2022
Closed
@wyli wyli mentioned this pull request Jan 18, 2022
Closed
7 tasks
@wyli wyli force-pushed the adds-base-writer-itk-writer branch from 5c05395 to e189fbf Compare January 18, 2022 22:09
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Jan 18, 2022

/build

Nic-Ma
Nic-Ma reviewed Jan 20, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Thanks for the quick implementation.
The overall logic looks good to me, except for the resample, as_channel_last logic.
Put some minor comments inline.

Thanks.

@wyli wyli force-pushed the adds-base-writer-itk-writer branch from 9e3dfa5 to 20d0bf9 Compare January 20, 2022 10:17
@wyli wyli mentioned this pull request Jan 21, 2022
Closed
wyli and others added 18 commits January 25, 2022 18:20
@wyli
temp spatial_resample
dad9365 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes resampling
0f07fc9 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes precisions
b09e568 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update dict version
0b4bc52 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes unit tests
996dacf 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
adds docs
2790ca4 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
copy grid for resampling
4ef11bf 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes unit tests
f65605e 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
remove normalize coordinates
b3011a7 
Signed-off-by: Wenqi Li <[email protected]>
@monai-bot @wyli
[MONAI] python code formatting
a1200dc 
Signed-off-by: monai-bot <[email protected]>
@wyli
try to fix Project-MONAI#3621 (Project-MONAI#3673)
7b91be4 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes typing
ec9aaf3 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes grid_sample, interpolate URLs
0ac197f 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge branch 'fixes-url' into add-spatial-resample
87ff30b 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
Merge remote-tracking branch 'upstream/dev' into add-spatial-resample
080f803 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
simplify norm_coords
aee76ee 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update docstring
0dc88af 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
update moveaxis
d044188 
Signed-off-by: Wenqi Li <[email protected]>
@wyli wyli force-pushed the adds-base-writer-itk-writer branch from cb6f9cc to dbf44ae Compare February 4, 2022 17:28
Nic-Ma
Nic-Ma reviewed Feb 5, 2022
View reviewed changes
@Nic-Ma Nic-Ma changed the title 3595 adds a base writer and an itk writer 3595 3766 adds a base writer and an itk writer Feb 5, 2022
@wyli wyli requested a review from Nic-Ma February 5, 2022 14:00
Nic-Ma
Nic-Ma approved these changes Feb 5, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Thanks for the quick update.
Looks good to me now, except for several minor comments inline.
@ericspod @rijobro Do you have any other comments?

Thanks.

@wyli wyli force-pushed the adds-base-writer-itk-writer branch from 55ac20e to 3961dfc Compare February 5, 2022 18:26
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 5, 2022 •
edited
Loading

/black

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 5, 2022 •
edited
Loading

Thanks for the quick update. Looks good to me now, except for several minor comments inline.

thanks Nic, I updated the PR for this use case of writing with a different spatial size:

# write as 2d image, channel-first
np_data = np.arange(48).reshape(3, 4, 4)
writer = ITKWriter(output_dtype=np.uint8)
writer.set_data_array(np_data, channel_dim=0)
writer.set_metadata({"spatial_shape": (5, 5)})
writer.write("test1.png")

could you review again to confirm?
also when writing the examples I found that

writer = ITKWriter(channel_dim=0)
writer.set_data_array(..., channel_dim=0)

will cause confusion, so I removed the constructor's channel_dim.

changes are in this commit 4e04de5.

@wyli wyli requested a review from Nic-Ma February 5, 2022 20:47
@wyli wyli force-pushed the adds-base-writer-itk-writer branch 2 times, most recently from 4b591f0 to 4e04de5 Compare February 5, 2022 21:33
@Nic-Ma @wyli
3765 Enhance create_multigpu_supervised_XXX for distributed (Projec…
f49a092 
…t-MONAI#3768)

* [DLMED] add check for devices

Signed-off-by: Nic Ma <[email protected]>

* [DLMED] update according to comments

Signed-off-by: Nic Ma <[email protected]>
@wyli wyli force-pushed the adds-base-writer-itk-writer branch from 4e04de5 to 531c2ec Compare February 5, 2022 21:59
@wyli wyli force-pushed the adds-base-writer-itk-writer branch from d3ad76f to 9a5fd2e Compare February 5, 2022 22:34
@wyli wyli mentioned this pull request Feb 5, 2022
Merged
7 tasks
Nic-Ma
Nic-Ma approved these changes Feb 7, 2022
View reviewed changes
Copy link
Copy Markdown
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 me except for 2 minor comments inline.

Thanks.

@wyli wyli force-pushed the adds-base-writer-itk-writer branch from 5736bbc to d0bdecc Compare February 7, 2022 08:55
@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 7, 2022

/build

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Feb 7, 2022

/build

@wyli wyli enabled auto-merge (squash) February 7, 2022 10:52
@wyli wyli merged commit afcf593 into Project-MONAI:dev Feb 7, 2022
@wyli wyli deleted the adds-base-writer-itk-writer branch April 27, 2022 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nic-Ma Nic-Ma Nic-Ma approved these changes

@rijobro rijobro Awaiting requested review from rijobro

@ericspod ericspod Awaiting requested review from ericspod ericspod is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wyli @ericspod @Nic-Ma @monai-bot