-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Make y optional in ignore_background function #8710
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
base: dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2cc0c16 to
864f902
Compare
864f902 to
fd724f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@monai/metrics/utils.py`:
- Around line 62-80: Update the ignore_background docstring to include a
Google-style Returns section describing the returned tuple (y_pred with
background channel removed when applicable, and y which may be None or have its
background removed) and the shapes/types returned; then add a unit test for the
function ignore_background that calls it with y=None (and representative y_pred
shapes for classification and segmentation) asserting it returns the expected
y_pred (background removed only when channels>1) and that y is still None,
ensuring no exceptions occur.
fd724f1 to
3954f5c
Compare
ericspod
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ytl0623 thanks for this fix, I think it's fine as a minor change with a couple of comments.
9dfa483 to
8d887a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@monai/metrics/utils.py`:
- Around line 80-91: Add unit tests covering both branches of the function that
uses y_pred, y, y_pred_out and y_out: (1) when y is None assert the function
returns a single tensor equal to y_pred (and when y_pred.shape[1] > 1 that it
returns y_pred[:,1:]); (2) when y is provided assert it returns a tuple
(y_pred_out, y_out) and that both are correctly sliced to [:,1:] when their
shape[1] > 1 and unchanged when shape[1]==1. Place tests to call the same
function containing this snippet, use small synthetic NdarrayTensor inputs to
validate shapes and values, and include assertions for returned types (single
tensor vs tuple).
Signed-off-by: ytl0623 <david89062388@gmail.com>
ff4bc92 to
665e2f0
Compare
Description
This PR addresses a TODO by refactoring the
ignore_backgroundutility function.Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.