Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the use of allow_smaller in CropForeground #6686

Merged
merged 8 commits into from
Jul 3, 2023

Conversation

KumoLiu
Copy link
Contributor

@KumoLiu KumoLiu commented Jul 2, 2023

Fixes #6685 .

Description

When allow_smaller=True, it allows the image size to be smaller than the box size and will pad it.
But in generate_spatial_bounding_box, it generates the wrong bounding box.

if allow_smaller:
min_d = max(min_d, 0)
max_d = min(max_d, spatial_size[di])

Change the default value of allow_smaller in CropForeground to False to be consistent with the previous behavior.

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.

Signed-off-by: KumoLiu <[email protected]>
Signed-off-by: KumoLiu <[email protected]>
@KumoLiu KumoLiu requested review from Nic-Ma, wyli and ericspod and removed request for Nic-Ma July 2, 2023 14:42
@wyli
Copy link
Contributor

wyli commented Jul 3, 2023

/build

@wyli wyli enabled auto-merge (squash) July 3, 2023 09:09
@wyli wyli merged commit dcdae0d into Project-MONAI:dev Jul 3, 2023
@KumoLiu KumoLiu deleted the cropforeground branch July 5, 2023 06:28
KumoLiu added a commit to KumoLiu/MONAI that referenced this pull request Jul 18, 2023
@KumoLiu KumoLiu mentioned this pull request Jul 18, 2023
7 tasks
wyli pushed a commit that referenced this pull request Jul 18, 2023
Fixes #6685.

- Revert #6686 without changing the meaning of `allow_smaller` in
`generate_spatial_bounding_box`.
- Update the docstring of the `allow_smaller` to make it more clear.
- Add `deprecated_arg_default`, then after 1.3, `CropForeground` with
pad by default even if the image edges are smaller than the final box
edges.


### 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).
- [ ] 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.

---------

Signed-off-by: KumoLiu <[email protected]>
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.

cropforeground with margin>0
2 participants