Prepare TensorFlow deprecation - add deeplabcut.api - #3382

Should this work on args as well?

yes should also be popped on the unified args. fixed now.

But modifying unified does not change the args which are then passed to TF right? And TF should not get engine

Thanks for staying alert. This was not fixed properly indeed for cases where engine is passed as positional arg. I've made it keyword-only now and added a validation step that runs at decoration time.

Note that making engine Keyword only changes the API! In fact I took the liberty to make more parameters keyword-only, since 12 positional args could get confusing easily. Let me know if you agree.

see 4fc2d18

Does this handle None?

good catch, None was missing from the normalization. I added it in _normalize_gputouse rather than dropping it, so users get feedback for incorrect usage of gputouse (+ device).

This will "break" the API docs in that the direct (still valid) imports like deeplabcut.analyze_videos will no longer be recognized.
I'm mentioning it, I doubt there is any easy clean fix for this, nor is it a big problem in the first place, but maybe worth mentioning.

(Dev docs are cleaner with the new version imo. I also checked again that all imports are still supported)

ok I agree that this needs to be improved. I implemented manual __getattr__ lazy loading following PEP 562, but that would have required some additions, like improving type checking to find modules before they are actually accessed as attribute.

After some more research, I think that lazy_loader is a good alternative (also used by napari, scikit-image) that keeps everything in one place (a .pyi stub) and works with type-checking. See #3459

More of a discussion point, but is it correct that
deeplabcut.modelzoo.video_inference
is now PT only, whereas

• deeplabcut.api.modelzoo_inference.video_inference_superanimal
• deeplabcut.video_inference_superanimal

have TF routing ? Maybe having a private torch only function and a single public TF wrapper would be cleaner ?
At the very least a direct import should warn if possible while keeping the current public call site, e.g.

# deeplabcut/modelzoo/video_inference.py

def _video_inference_superanimal_pytorch(...):
    ...

# deeplabcut/api/modelzoo_inference.py

@with_tensorflow_fallback(
    when=lambda params: params.get("model_name") in _TENSORFLOW_MODELS,
    ...
)
def video_inference_superanimal(...):
    from deeplabcut.modelzoo.video_inference import (
        _video_inference_superanimal_pytorch,
    )

    return _video_inference_superanimal_pytorch(...)
# deeplabcut/modelzoo/video_inference.py

def video_inference_superanimal(*args, **kwargs):
    warnings.warn(
        "Import video_inference_superanimal from deeplabcut "
        "or deeplabcut.api instead.",
        DLCDeprecationWarning,
        stacklevel=2,
    )

    from deeplabcut.api.modelzoo_inference import (
        video_inference_superanimal as public_api,
    )

    return public_api(*args, **kwargs)

Also is dlcrnet the only TF model ?
Since the old implementation selected the backend through models_to_framework.json, while the new router appears to select TensorFlow only when model_name == "dlcrnet".

good point, and I think in a broader sense we should streamline this for all functions. Currently pose_estimation_pytorch is a clear exported module on its own, which is re-exported in deeplabcut.api + at the top-level now (including the tf-routing decorator). Docstrings are largely duplicated.

I think after the full stack is merged we should revisit this and see if we need to restructure the exports / module locations a bit. "

For now I've addressed your comment directly in 5cae12f, but keeping this comment open for later discussion.

Just to be sure, this tests _resolve_legacy_kwargs, which is torch only right? Could we add tests that route to TF while both a legacy and current name are provided?

Maybe add one or two integration tests using mocked read_config and get_shuffle_engine, for example analyze_videos and export_model(cfg_path=...)?
To catch issues with real API funcs.

good suggestion. See 58bda12

Maybe add where relevant:

import deeplabcut.api

@pytest.mark.parametrize("name", deeplabcut.api.__all__)
def test_migrated_export_available_from_public_facades(name):
    import deeplabcut
    import deeplabcut.api

    assert name in deeplabcut.__all__
    assert name in deeplabcut.api.__all__
    assert getattr(deeplabcut, name) is getattr(deeplabcut.api, name)