Skip to content

fix: correct return type annotation of get_nsys_entrypoint - #568

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/get-nsys-entrypoint-return-type
Open

fix: correct return type annotation of get_nsys_entrypoint#568
andrewwhitecdw wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/get-nsys-entrypoint-return-type

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

Bug

get_nsys_entrypoint returns a tuple ("nsys", ""), but its annotation was str. This mismatched the implementation and callers that unpack the result.

Fix

Update the return type annotation to tuple[str, str].

Test

The existing TestExecutor::test_get_nsys_entrypoint already asserts the tuple return value.

Verification

uv run pytest test/core/execution/test_base.py::TestExecutor::test_get_nsys_entrypoint -v passes. uv run --group lint ruff check ... and ruff format --check ... pass.

get_nsys_entrypoint returns a tuple (nsys, ), but its annotation was str.

Update the annotation to tuple[str, str] to match the implementation and

callers that unpack the result.

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

Copy link
Copy Markdown
Contributor

PR #568 — needs attention
Blocking finding: nemo_run/core/execution/slurm.py:564-570 still annotates its override as returning str, although it returns the same two-string tuple as the corrected base method and is tuple-unpacked by nemo_run/run/torchx_backend/packaging.py:253. Update the override to tuple[str, str] so concrete SlurmExecutor use is typed correctly.
Non-blocking findings: None.
Validation gaps: Runtime tuple tests exist, but they do not catch this annotation mismatch. Only DCO is reported.

@ko3n1g

ko3n1g commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

hey @andrewwhitecdw, could you fix the type hint? I'll get it merged afterwards. Thanks for the contribution!

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the waiting-on-customer Waiting on the original author to respond label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-request waiting-on-customer Waiting on the original author to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants