Prepare TensorFlow deprecation - add deeplabcut.api - #3382
Conversation
|
(Needs formatted docstrings and typed configs still) |
C-Achard
left a comment
There was a problem hiding this comment.
Good start,I really like the design overall, I just think some of the current routing might need to be made safer; happy to discuss if needed!
Apply `renamed_params` aliases (e.g., `cfg_path` → `config`) to the bound `unified` dict before `_resolve_engine` and `when` inspect it, so they always see canonical parameter names.
|
@C-Achard thanks for your review. I've made the tf_routing more robust now. I've addressed all your comments, and added more tests. I will do another iteration next week to see if there are more tests to be added, and try it out in practice a low. It is a high-impact PR, so I agree that it should be well-tested. |
…d stale references
C-Achard
left a comment
There was a problem hiding this comment.
Just a few preliminary comments while going through this again, let's perhaps discuss which areas would benefit from further review/testing as it has grown quite a bit.
|
|
||
|
|
||
| @deprecated(replacement="deeplabcut.core.config.ProjectConfig", since="3.1") | ||
| @deprecated(replacement="deeplabcut.core.config.ProjectConfig", since="3.0.1") |
There was a problem hiding this comment.
A small question/design curiosity when I see this, should we have a file that defines several consts with version numbers mapping to deprecation, so that whenever there is a round of deprecating something, there is one single source for the version, minimizing the need to globally search for version numbers, and avoiding changing version numbers for an unrelated round of deprecation or on the wrong API?
| def __init__(self, root, parent, h1_description): | ||
| super().__init__(root, parent, h1_description) | ||
|
|
||
| self._reload_timer = QTimer(self) |
There was a problem hiding this comment.
Should these defs be moved to the base class?
| def __init__(self, root, parent, h1_description): | ||
| super().__init__(root, parent, h1_description) | ||
|
|
||
| self._reload_timer = QTimer(self) |
There was a problem hiding this comment.
As mentioned elsewhere, might be best moved to the base class instead ? Even if disabled by default
Preparation for full TensorFlow deprecation
deeplabcut.compatand split into PyTorch-onlydeeplabcut.apiand TF-onlydeeplabcut.tensorflow_compatBackground
TensorFlow support will be dropped soon and a clean preparation is needed to make the transition smooth. Although the pose_estimation code was already cleanly branched in
deeplabcut/compat.py, this brought the disadvantage that the main entrypoint for DeepLabCut is actually mixed API, which was not ideal to test and remains a small obstacle to eventually remove TF cleanly.This PR makes a large step towards clean TF deprecation, by splitting the
compatmodule into our new clean user-facing API (Pytorch-only) with a fallback for TensorFlow instead of mixed API. It introduces some new packages and removesdeeplabcut.compatas the entry point for the public API.New
deeplabcut.api: the new home for user-facing API functions. Currently covers all pose estimation functions (train_network, evaluate_network, analyze_videos, analyze_images, etc.), but can cover other main API as well.deeplabcut.tensorflow_compat— an isolated compatibility layer that holds the TensorFlow-specific branches, split out of compat.py. This module is explicitly marked as temporary and will be deleted wholesale when TF support is dropped.deeplabcut.api._tf_routing— a new decorator-based routing system (@with_tensorflow_fallback) that automatically intercepts calls to functions in deeplabcut.api, resolves the engine and delegates todeeplabcut.tensorflow_compathandling TF-only legacy kwargs. ADLCDeprecationWarningis emitted to inform user about the migration.Next steps
This PR intentionally scopes to pose estimation only. (I included
generate_training_datasetas well first, it's probably better to work in incremental changes, rather than one big PR).Several other areas still go through compat or have TF branches inline and need similar treatment in follow-up PRs:
generate_training_dataset) — the largest remaining concern.deeplabcut.utils— scattered TF-conditional helpers.deeplabcut.modelzoo— TF-specific download / inference paths.The goal is that every user-facing function ultimately lives under deeplabcut.api with a clean, PyTorch-only signature, while all TF code is quarantined in deeplabcut.tensorflow_compat and removable in a single future commit.
Test coverage
tests/api/test_pose_estimation.py(480 lines) — smoke tests for the new deeplabcut.api pose estimation surface: parameter forwarding, engine routing, legacy kwarg handling.tests/api/test_tf_routing.py(360 lines) — unit tests for the _tf_routing machinery: engine resolution, renamed/dropped param warnings, gputouse normalisation, multi-shuffle validation.tests/core/test_visualization.py(191 lines) — new coverage for deeplabcut.core.visualization helpers.tests/pose_estimation_pytorch/apis/test_apis_utils.py(102 lines) — new coverage for PyTorch API utilities.