Project-MONAI / Project-MONAI/MONAI
Enforce kwargs only for most methods and classes
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 8.7k
- Forks
- 1.6k
- Avg merge
- 5d 1h
- Merged PRs (30d)
- 20
Description
Is your feature request related to a problem? Please describe.
Often, when refactoring models and code, parameters become obsolete, or new ones are added. As MONAI tries to maintain backward compatibility, any arguments that can be used as positional arguments make changes like these often very difficult.
IMO most networks, transforms etc will and should be called with keyword arguments to help readability. And in the case of monai configs, they are called with kwargs anyway.
E.g.
We want to remove save_mem from the following class:
class GroupNorm3D(nn.GroupNorm):
def __init__(
self,
num_groups: int,
num_channels: int,
eps: float = 1e-5,
affine: bool = True,
save_mem: bool = True,
norm_float16: bool = False,
print_info: bool = False,
):
Since there is a public interface, users could call this using GroupNorm3D(10,4,args.eps,args.affine,args.args.save_mem)
When we now remove the argument save_mem, we have no way of determining whether the user actually wanted to call the class with the old save_mem parameter or the norm_float16 parameter at the now 5th position.
If we use kw-only args, this would not be a problem and the deprecate_arg decorator could be applied without issue.
Example:
class GroupNorm3D(nn.GroupNorm):
def __init__(
self,
num_groups: int,
num_channels: int,
*,
eps: float = 1e-5,
affine: bool = True,
save_mem: bool = True,
norm_float16: bool = False,
print_info: bool = False,
):
or even not allowing any positional arguments, which I think would be best for most classes, but not for utility functions.
class GroupNorm3D(nn.GroupNorm):
def __init__(
self,
*,
num_groups: int,
num_channels: int,
eps: float = 1e-5,
affine: bool = True,
save_mem: bool = True,
norm_float16: bool = False,
print_info: bool = False,
):
I think MONAI should consider making all new (and eventually existing) networks, transforms, etc, that do not have an obvious need for positional arguments to only use kwargs-only. This would force readable calls and an easier-to-maintain code base.
This should also apply to any special arguments for utility functions, etc, that will not always be used or whose position is not important on the call.
This could be either enforced during review or with flake8/ruff rules.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
No files or tests are named. Start by surveying network, transform, class, and utility-function signatures to determine the scope, then compare review enforcement with flake8 or ruff rules. Done requires an agreed policy for positional arguments and a defined path for applying or checking it across the affected APIs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- backend-api-design, developer-experience, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100