Deprecate legacy training APIs

[holgerroth]

Summary

• add a reusable warn_deprecated helper that preserves the existing always-visible deprecation behavior
• emit deprecation warnings when ModelLearner and both NPTrainer variants are instantiated
• update ModelLearner docs to steer new projects to Job Recipe + Client API
• add tests that warnings fire while class identity/subclass behavior stays intact

Existing example usage check

• Checked examples/ for live imports/config references to ModelLearner, ModelLearnerExecutor, and both NPTrainer paths.
• No current examples instantiate or configure these legacy APIs; matches are historical comments pointing to old removed model-learner examples.
• Legacy docs and job_templates/ still reference these APIs, so this PR warns on use but does not remove or break backward-compatible loading.

Validation

• pytest tests/unit_test/fuel/utils/deprecated_test.py tests/unit_test/app_common/abstract/model_learner_test.py tests/unit_test/app_common/np/np_trainer_test.py tests/unit_test/app_common/ccwf/ccwf_np_trainer_test.py tests/unit_test/job_config/fed_job_test.py -q
• git diff --check

[holgerroth]

@greptileai review

[greptile-apps]

Greptile Summary

This PR deprecates the legacy ModelLearner and NPTrainer training APIs by adding a reusable warn_deprecated helper that always fires a DeprecationWarning regardless of global filter state, and wires it into the __init__ of each legacy class. Documentation is updated to steer new users to the Job Recipe + Client API, and unit tests are added to verify the warnings fire without breaking class identity or subclass behavior.

• nvflare/fuel/utils/deprecated.py: Introduces warn_deprecated(message, stacklevel) guarded by a module-level threading.Lock with a nested warnings.catch_warnings() block so the warning always fires unconditionally, even when a caller has set a global "ignore" filter; the deprecated decorator is refactored to delegate to this helper.
• model_learner.py / np_trainer.py (both variants): Each __init__ calls warn_deprecated(..., stacklevel=3) before super().__init__(), so the warning points at the instantiation site rather than the __init__ body.
• Tests: New test files for ModelLearner, both NPTrainer variants, and warn_deprecated directly; updated fed_job_test.py wraps ModelLearner() calls in pytest.warns to silence the new warnings.

Confidence Score: 5/5

Safe to merge — the change is purely additive, emitting a DeprecationWarning on instantiation of legacy classes without altering any functional behavior.

The warn_deprecated helper is narrowly scoped: it acquires a module-level lock, temporarily overrides the warning filter inside a catch_warnings() block, and restores global state on exit. The thread-safety concern and stacklevel contract from earlier rounds have both been addressed. Every changed class retains full backward compatibility, all constructor parameters have defaults, and the new tests cover direct instantiation, subclassing, and the helper itself. No logic, data path, or security boundary is modified.

No files require special attention.

Important Files Changed

Filename	Overview
nvflare/fuel/utils/deprecated.py	Adds warn_deprecated with thread-safe lock + catch_warnings() to always fire DeprecationWarning; refactors deprecated decorator to delegate to it. Logic and stacklevel contracts are correct.
nvflare/app_common/abstract/model_learner.py	Emits deprecation warning in __init__ with stacklevel=3 before super().__init__(). Correct for direct instantiation; known stacklevel offset for subclasses that override __init__ is an accepted trade-off.
nvflare/app_common/np/np_trainer.py	Adds deprecation warning to NPTrainer.__init__ with stacklevel=3; all constructor params have defaults so zero-arg instantiation in tests is valid.
nvflare/app_common/ccwf/comps/np_trainer.py	Parallel change to the CCWF NPTrainer variant; identical pattern and same analysis as the non-CCWF variant.
tests/unit_test/fuel/utils/deprecated_test.py	New test_warn_deprecated uses warnings.catch_warnings(record=True) and correctly asserts message, category, and filename; co-exists safely with the inner catch_warnings() in warn_deprecated.
tests/unit_test/app_common/abstract/model_learner_test.py	Tests direct instantiation and subclass instantiation; both cases correctly expect a DeprecationWarning from ModelLearner.__init__.
tests/unit_test/job_config/fed_job_test.py	Extracts _create_model_learner() helper that wraps instantiation in pytest.warns so the two existing ModelLearner() call sites no longer produce unexpected-warning failures.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["ModelLearner.__init__ / NPTrainer.__init__"] -->|"calls stacklevel=3"| B["warn_deprecated"]
    J["deprecated decorator new_func"] -->|"calls stacklevel=3"| B
    B --> C["Acquire _DEPRECATION_WARNING_LOCK"]
    C --> D["warnings.catch_warnings enter"]
    D --> E["simplefilter always DeprecationWarning"]
    E --> F["warnings.warn DeprecationWarning"]
    F --> G["Restore saved filter state"]
    G --> H["Release lock"]
    H --> I["super init continues normally"]

    style B fill:#f9a,stroke:#c66
    style C fill:#adf,stroke:#06c
    style F fill:#ffd,stroke:#aa0

Loading

_{Reviews (2): Last reviewed commit: "Clarify legacy API warning text" | Re-trigger Greptile}

[YuanTingHsieh]

do we want to also deprecate

• nvflare.app_common.np.np_formatter.NPFormatter
• nvflare.app_common.ccwf.comps.np_file_model_persistor.NPFileModelPersistor

[holgerroth]

I checked current usage before expanding scope. NPFormatter and NPFileModelPersistor are still referenced by legacy job templates/docs and integration-test fixtures, so I left them out of this PR rather than broadening the warning surface. This PR now keeps the deprecation to ModelLearner and both NPTrainer classes only.

[YuanTingHsieh]

Our plan is to deprecate them in 2.9 and remove in the 2.10?

[holgerroth]

Not sure if we need to remove them if they are still supported. Just do not recommend building on top of it.

[holgerroth]

Updated in 0bcd08751 to avoid implying a scheduled removal: the warning now says these APIs are deprecated but remain supported for backward compatibility, and points users to Recipe API + Client API for new projects. This matches the intent of not recommending new work on top of them while keeping existing usage supported.

[holgerroth]

@greptileai review again