Skip to content

fix: preserve falsy primitive args in Config/Partial binding - #566

Merged
ko3n1g merged 3 commits into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/config-arg-truthiness
Aug 7, 2026
Merged

fix: preserve falsy primitive args in Config/Partial binding#566
ko3n1g merged 3 commits into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/config-arg-truthiness

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

Bug

_construct_args used if arg: to decide whether to copy a keyword argument, which treated 0, False, and "" as missing.

Fix

Use if arg is not None: so legitimate falsy values are preserved during binding.

Test

Added TestPartial::test_falsy_primitive_args_are_preserved, which verifies that 0, False, and empty-string args survive run.Partial construction and fdl.build.

Verification

uv run pytest test/test_config.py::TestPartial::test_falsy_primitive_args_are_preserved -v passes.
uv run --group lint ruff check nemo_run/config.py test/test_config.py and ruff format --check ... pass.

_construct_args used  to decide whether to copy a keyword

argument, which treated 0, False, and '' as missing. Use

so legitimate falsy values are preserved during binding.

Adds a regression test verifying that 0, False, and empty string args

survive Partial construction and build.

Signed-off-by: Andrew White <andrewh@cdw.com>

Copy link
Copy Markdown
Contributor

PR #566 — LGTM
Blocking findings: None.
Non-blocking findings: None. Changing the presence check at nemo_run/config.py:577-587 to arg is not None preserves 0, False, and "" without changing the special handling for omitted None. test/test_config.py:229-234 exercises all three values through Partial and fdl.build.
Validation gaps: Only successful DCO is reported; no test or lint check is visible.

@ko3n1g
ko3n1g enabled auto-merge (squash) August 7, 2026 11:45
@ko3n1g
ko3n1g merged commit 514d076 into NVIDIA-NeMo:main Aug 7, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants