build(deps): make mlflow and wandb optional extras - #3785
Conversation
8cb3f0d to
d10f565
Compare
|
Hi @akoumpa @HuiyingLi — could you approve the CI workflows for this community PR when you get a chance? This removes Closes #3783. Happy to address any review feedback. Thanks! |
|
/ok to test d10f565 |
d10f565 to
5ac9562
Compare
|
@akoumpa Updated both lockfiles ( |
5ac9562 to
feeb7ab
Compare
e6eab03 to
a2f9e11
Compare
|
/ok to test a2f9e11 |
|
@akoumpa is the CI expected to take this much time? It seems to be stuck in queue. Do lemme know if I am missing anything |
|
Hi @piyushumate apologies for the delay; the CI workers are oversubscribed, thus the delay :( I'll mention this to our automation folks to explore any potential solution -- in the meantime, i'll open a separate PR to run CI to leverage separate CI worker queue, if that passes, i'll FM your PR. |
|
@akoumpa CI passed, please lemme know if anything else is needed. Thanks! |
|
Hi @piyushumate can you take a look at the conflicts? i can rerun ci then |
mlflow and wandb are experiment-tracking backends that are opt-in via YAML
config. Listing them as hard core dependencies causes two problems:
1. mlflow (full) conflicts with mlflow-skinny — mutually exclusive PyPI
packages. Environments shipping mlflow-skinny (sagemaker-mlflow,
kedro-mlflow, managed ML platforms) hard-block on install.
2. Both packages pull in heavy transitive deps unconditionally even for users
who never configure a logger.
Changes:
- pyproject.toml: remove wandb and mlflow from dependencies[]; add:
[mlflow] = mlflow-skinny (no conflict, full tracking API)
[mlflow-full] = mlflow (UI, SQL backend, pyarrow, etc.)
[wandb] = wandb>=0.28.0
[tracking] = nemo-automodel[mlflow] + nemo-automodel[wandb]
Wire [tracking] into [all]. Fix composite extras to use canonical
distribution name nemo-automodel (hyphens, not nemo_automodel underscores).
- loggers.py / wandb_utils.py: use safe_import() / safe_import_from() from
nemo_automodel.shared.import_utils per CLAUDE.md coding-style rule.
Remove inline try/except ImportError guards.
- 8 recipe files (llm/train_ft, llm/kd, llm/train_seq_cls, dllm/train_ft,
diffusion/train, vlm/finetune, vlm/kd, multimodal/finetune): replace
top-level import wandb / import mlflow with module-level safe_import calls
(returns _HAS_WANDB/_HAS_MLFLOW boolean + module-or-placeholder).
Prefix every wandb.run is not None guard with _HAS_WANDB and, and every
mlflow.active_run() is not None with _HAS_MLFLOW and.
- tests/unit_tests/loggers/test_optional_logger_extras.py [NEW]: 13 unit
tests covering instantiation without extras, UnavailableError on build()
with absent extras, pyproject.toml extras schema, and recipe module
_HAS_* sentinel presence.
- ruff format + ruff check --fix applied to all changed files.
Closes NVIDIA-NeMo#3783
Precedent: sagemaker-mlflow PR NVIDIA-NeMo#22 applied the identical fix.
Signed-off-by: piyushumate <piyushumate@users.noreply.github.com>
|
@akoumpa, conflicts have been resolved and the branch is rebased onto latest main. Both lockfiles have been regenerated and verified locally, and all linters and tests pass. Ready for CI. Thanks! |
|
/ok to test 080ad40 |
|
@akoumpa seems like CI oversubscribed? (nvm , it passed) |
What does this PR do?
Make
mlflowandwandboptional extras instead of hard core dependencies. Closes #3783.Changelog
pyproject.toml: removemlflowandwandb>=0.28.0fromdependencies[]; add[mlflow](→mlflow-skinny),[mlflow-full](→ fullmlflow),[wandb], and[tracking](meta-extra) in[project.optional-dependencies]; wire[tracking]into[all]; fix all composite extras to use canonical distribution namenemo-automodel(hyphens)nemo_automodel/shared/import_utils.py(existing):safe_import/safe_import_fromnow used for all optional-dep guards perCLAUDE.mdcoding style rulecomponents/loggers/loggers.py: module-levelsafe_importforwandb/mlflow; remove inlinetry/except ImportErrorfromWandbConfig.build()andMLflowConfig.build()components/loggers/wandb_utils.py: remove inlinetry/exceptininit_wandb_run; defer error tosafe_importplaceholder at call time;uv-based install hintllm/train_ft,llm/kd,llm/train_seq_cls,dllm/train_ft,diffusion/train,vlm/finetune,vlm/kd,multimodal/finetune): replace top-levelimport wandb/import mlflowwith module-levelsafe_importcalls (returns boolean flag + module-or-placeholder); prefix everywandb.run is not Noneguard with_HAS_WANDB andand everymlflow.active_run() is not Nonewith_HAS_MLFLOW andto short-circuit safely without installing the extrastests/unit_tests/loggers/test_optional_logger_extras.py[NEW]: unit tests covering instantiation without extras,UnavailableErroronbuild()with absent extras, pyproject.toml extras schema, and recipe module_HAS_*sentinel presenceruff format+ruff check --fixapplied to all changed filesInstall patterns
Before your PR is "Ready for review"
Pre checks:
tests/unit_tests/loggers/test_optional_logger_extras.pyAdditional Information