Skip to content

3357 Add wrap_sequence to several transforms#3358

Merged
Nic-Ma merged 13 commits intoProject-MONAI:devfrom
Nic-Ma:3357-add-wrap_sequence
Nov 19, 2021
Merged

3357 Add wrap_sequence to several transforms#3358
Nic-Ma merged 13 commits intoProject-MONAI:devfrom
Nic-Ma:3357-add-wrap_sequence

Conversation

@Nic-Ma
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma commented Nov 18, 2021

Fixes #3357 .

Description

This PR added the wrap_sequence parameter to several transforms.

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

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/black

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/black

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021 •
edited
Loading

Hi @rijobro @wyli @yiheng-wang-nv ,

Please note that in this PR I set different default values of wrap_sequence for utils(False) and transforms(True), following our current pattern in: https://github.com/Project-MONAI/MONAI/blob/dev/monai/utils/type_conversion.py and https://github.com/Project-MONAI/MONAI/blob/dev/monai/transforms/utility/array.py#L356
I think it matches the common usage, we always convert a list of data(like classification labels) to a numpy array or tensor in the transforms.
Could you please help review it?

Thanks in advance.

@Nic-Ma
[DLMED] fix tests
658d709 
Signed-off-by: Nic Ma <[email protected]>
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

@Nic-Ma
[DLMED] fix typo
58cd0c6 
Signed-off-by: Nic Ma <[email protected]>
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

yiheng-wang-nv
yiheng-wang-nv reviewed Nov 18, 2021
View reviewed changes
@Nic-Ma Nic-Ma force-pushed the 3357-add-wrap_sequence branch from 48f9db8 to e70e645 Compare November 18, 2021 15:57
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/black

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

/build

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Nov 18, 2021

Hi @wyli @yiheng-wang-nv ,

Do you have any other concerns about this PR?

Thanks in advance.

yiheng-wang-nv
yiheng-wang-nv approved these changes Nov 19, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@yiheng-wang-nv yiheng-wang-nv left a comment

Choose a reason for hiding this comment

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

Thanks, looks good on me.

@Nic-Ma Nic-Ma merged commit 79bca25 into Project-MONAI:dev Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiheng-wang-nv yiheng-wang-nv yiheng-wang-nv approved these changes

@rijobro rijobro Awaiting requested review from rijobro

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

@wyli wyli Awaiting requested review from wyli

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing wrap_sequence in ToNumpy and EnsureType

2 participants

@Nic-Ma @yiheng-wang-nv