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

Renaming include_background #6930

Closed
wyli opened this issue Sep 2, 2023 Discussed in #6915 · 2 comments
Closed

Renaming include_background #6930

wyli opened this issue Sep 2, 2023 Discussed in #6915 · 2 comments

Comments

@wyli
Copy link
Contributor

wyli commented Sep 2, 2023

Discussed in #6915

Originally posted by ellisdg August 30, 2023
In many of the loss classes and metrics in MONAI, the creators have put in an argument named include_background. To my understanding, what this argument does is include the first channel in the computations, if True, and exclude the first channel in the computations, if False.

I have a few issues with the naming of the include_background argument:

  1. The name is ambiguous and does not reflect what the argument actually does.
  2. The background channel could be any of the channels, and does not necessarily have to be the first channel.
  3. When there is no background channel, you should specify include_background=True, which is counter-intuitive because there is no background to be included.

Therefore, why isn't the argument called include_first_channel instead? This name says what the include_background argument actually does.

would be great to rename it to include_zeroth_channel or include_channel_zero, while keeping the existing name for backward compatibility for two versions.

@myron
Copy link
Collaborator

myron commented Sep 29, 2023

I think it almost does not matter, to me, the existing name and the new proposed name both are clear. I'll be ok either way, but we've already got used to the current names, which were there for years..
#7056 (comment)

@vikashg
Copy link

vikashg commented Jan 4, 2024

closing will reopen if further discussion is needed.

@vikashg vikashg closed this as completed Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants