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.

Sign up for GitHub

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

Jump to bottom

config component shouldn't allow duplicated arguments #5471

Closed
wyli opened this issue Nov 4, 2022 · 0 comments · Fixed by #6252
Closed

config component shouldn't allow duplicated arguments #5471

wyli opened this issue Nov 4, 2022 · 0 comments · Fixed by #6252
Labels
Contribution wanted Feature request

Comments

@wyli
Copy link
Contributor

wyli commented Nov 4, 2022

Describe the bug

"dataset": {
    "_target_": "CacheDataset",
    "num_workers": 0,
    "num_workers": 4
},

in this configuration example, the duplicated num_workers is currently quietly allowed, which is error-prone in my understanding

cc @Nic-Ma
@kretes kretes mentioned this issue Mar 28, 2023
Merged
5 tasks
wyli pushed a commit that referenced this issue Mar 29, 2023
@kretes
warn or raise ValueError on duplicated key in json/yaml config (#6252)
45a398a 
Fixes #5471 .

### Description

[WIP]
Monai will now raise an error whenever there is a duplicated key in any
of the config json files.

TBD:
- support similiar thing when reading from yaml

Things to discuss:
- do we want to raise an Error or do we just want a warning message?
- This will raise with the first duplicate encountered, not with a full
list, but I think that's a fair & simple approach.

### 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.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`. (I couldn't run them locally due to OOM)
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.

~~Documentation updated, tested `make html` command in the `docs/`
folder.~~ I think that's a safeguard that doesn't require explicit
documentation

---------

Signed-off-by: Tomasz Bartczak <kretesenator@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Contribution wanted Feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

warn or raise ValueError on duplicated key in json/yaml config kretes/MONAI
1 participant
@wyli