Skip to content

Fix omni3 CLI test collection and restore super3/omni3 command registration#294

Open
temujinkz wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
temujinkz:fix-omni3-test-cli-imports
Open

Fix omni3 CLI test collection and restore super3/omni3 command registration#294
temujinkz wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
temujinkz:fix-omni3-test-cli-imports

Conversation

@temujinkz

Copy link
Copy Markdown
Contributor

Fixes #284

The test file imported the omni3 build dispatcher that was removed when container builds moved to the shared nemotron kit slurm build command, so the whole file died at collection. Dropped the stale build tests and kept the job-name and app-structure tests that still point at real code.

Getting the file running again turned up a second bug: super3's data prep group still imports META, rl from the rl module that became a subgroup package. That import error silently knocks both super3 and omni3 out of the CLI, since group registration swallows failures. Mounting rl_app the way omni3 does fixes both groups.

Verified on Python 3.12 with full deps: the file goes from a collection error to 20 passed, and nemotron super3 data prep --help and nemotron omni3 --help both show their commands again. Thanks @andrewwhitecdw for the detailed report.

tests/recipes/omni3/test_cli.py still imported the omni3 build
dispatcher that was removed when container builds moved to the shared
kit slurm build command, so pytest died at collection. Drop the stale
build tests and keep the job-name and app-structure checks.

Getting the file running again exposed a second problem: super3's data
prep group still imports META/rl from the rl module that became a
subgroup package. That import error silently drops both super3 and
omni3 from the CLI because group registration swallows failures.
Mount rl_app the way omni3 does.

Fixes NVIDIA-NeMo#284

Signed-off-by: temujinkz <ttalkenov@gmail.com>
@temujinkz

Copy link
Copy Markdown
Contributor Author

Heads up: #283 also fixes the super3 rl registration in _typer_group.py, so that part overlaps (it was opened first). Happy to rebase and drop that hunk if #283 lands before this one, the omni3 test collection fix here is independent either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests: tests/recipes/omni3/test_cli.py imports missing nemotron.cli.commands.omni3.build module

1 participant