Skip to content

array transforms inherit Transform#2219

Merged
Nic-Ma merged 1 commit into
Project-MONAI:devfrom
wyli:revise-transform-parents
May 20, 2021
Merged

array transforms inherit Transform#2219
Nic-Ma merged 1 commit into
Project-MONAI:devfrom
wyli:revise-transform-parents

Conversation

@wyli

@wyli wyli commented May 19, 2021

Copy link
Copy Markdown
Contributor

Signed-off-by: Wenqi Li wenqil@nvidia.com

minor fixes for the array transforms, follow-up of #1952

Status

Ready

Types of changes

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

Signed-off-by: Wenqi Li <wenqil@nvidia.com>
@wyli wyli requested review from Nic-Ma, ericspod and rijobro May 19, 2021 22:43
@Nic-Ma Nic-Ma merged commit ba03150 into Project-MONAI:dev May 20, 2021
@rijobro

rijobro commented May 20, 2021

Copy link
Copy Markdown
Contributor

All of these should have probable inherited from RandomizableTransform, as opposed to Randomizable and Transform independently. I'll create a follow up PR.

@wyli wyli deleted the revise-transform-parents branch May 20, 2021 08:52
@wyli

wyli commented May 20, 2021

Copy link
Copy Markdown
Contributor Author

but the concepts of do_transform and prob from RandomizableTransform are not needed in these transforms, I feel in heriting that API is not needed

@rijobro

rijobro commented May 20, 2021

Copy link
Copy Markdown
Contributor

Ah I see. Ok, thanks!

@rijobro

rijobro commented May 20, 2021

Copy link
Copy Markdown
Contributor

Although perhaps for the sake of continuity across all of our random transforms, it would be nice if they also had a prob argument. This wouldn't be backwards compatible though, so probably not worth it.

wyli added a commit that referenced this pull request May 26, 2021
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
wyli added a commit that referenced this pull request May 26, 2021
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
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