Skip to content

Conversation

@bhashemian
Copy link
Member

@bhashemian bhashemian commented Nov 21, 2022

Fixes #5553

Description

Promote specified children of specified items to the level of item.

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

@bhashemian
Copy link
Member Author

bhashemian commented Nov 21, 2022

A use case can be found here for Project-MONAI/tutorials#1055

@bhashemian bhashemian marked this pull request as ready for review November 22, 2022 03:47
@bhashemian bhashemian requested review from KumoLiu and myron November 22, 2022 14:01
@bhashemian
Copy link
Member Author

@wyli @Nic-Ma could you please help to review this PR as the progress of testing hovernet integration with MONAI label relies on this?

Copy link
Collaborator

@myron myron left a comment

Choose a reason for hiding this comment

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

Hi @drbeh , it seems good to me, but the name PromoteChildItemsd() is not very intuitive. You can consider calling it
MergeSubKeys(sub_keys=...) or FlattenSubKeys(sub_keys=..). And if sub_keys is not provided, merge all of them.

but this is minor, should not a be stopper for this issue..

@wyli
Copy link
Contributor

wyli commented Nov 23, 2022

agree with @myron, I think PromoteChildItemsd is not very intuitive. also, how to deal with duplicated sub-keys such as {"a": {"a": 1, "b": 2}}?

@bhashemian bhashemian requested a review from myron November 23, 2022 14:59
@bhashemian
Copy link
Member Author

@myron @wyli I have change the name, added the support to only specify the key, and handle situations where the same key exists at the top level.

@bhashemian bhashemian changed the title Implement PromoteChildItemsd Implement FlattenSubKeysd Nov 23, 2022
Copy link
Contributor

@wyli wyli 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, please fix the typos I'll merge this

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@bhashemian bhashemian enabled auto-merge (squash) November 23, 2022 16:39
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Nov 23, 2022

/build

@bhashemian bhashemian merged commit 86f2573 into Project-MONAI:dev Nov 23, 2022
@bhashemian bhashemian deleted the promote-child-itemd branch November 23, 2022 17:40
wyli pushed a commit to bhashemian/MONAI that referenced this pull request Nov 23, 2022
Fixes Project-MONAI#5553 

### Description

Promote specified children of specified items to the level of item.

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [x] New tests added to cover the changes.
- [x] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.
- [x] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.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.

Promote Child Items to Top Level

3 participants