add stub-driven lazy imports using lazy_loader - #3459
deruyter92 wants to merge 122 commits into
Conversation
There was a problem hiding this comment.
Will this ever run in CI or is it meant as local only?
There was a problem hiding this comment.
A few notes:
- this fixture is now moved to tools instead of tests, together with the actual tool
test_type_stub.pythat uses it. This enables running it as a separate tool without deeplabcut or any dependencies installed. - it is now hooked into CI via a separate lightweight type checking test see 6b3c8e5
There are also some tests that check if the imports resolve at runtime defined in test_top_level_api.py. See other comment below. Let me know if you think this is excessive.
|
|
||
|
|
||
| @pytest.mark.skipif( | ||
| not (_module_available("torch") and _module_available("PySide6")), |
There was a problem hiding this comment.
The skip condition does not represent the full optional dependency set, right? With PySide6 installed but missing another GUI dep, this would run. Can we mark this test in the CI test selector maybe? Or maybe there are other minimal solutions that don't require copying the deps from pyproject at all
There was a problem hiding this comment.
I am reading the GUI deps from pyproject.toml now and removed the skipif everything runs now, and those modules that require GUI dependencies are excepted in case these dependencies are missing.
Note that we now have two runtime tests that resolve all exports (+ a type-checking tool that can run in CI to test the stubs).
let me know if you think this is excessive
|
Note: In |
|
Also the import all calls in |
|
For the GUI, could we disable lazy imports ? Having the interface freeze on imports makes it seem unresponsive, in this case the previous behavior of importing all on launch may be a bit more user-friendly |
|
@C-Achard, thanks for your review comments.
With the current implementation, deeplabcut.version is eagerly bound at
Done, thanks
I've added a GUI warm-up now, that imports all public deeplabcut exports + heavy imports on a deamon thread. |
…kers update deprecation marker versions (TF deprecation since 3.1, removed in 3.2)
Replace DeepLabCut's hand-maintained lazy export maps (_API_EXPORTS_MAP, _OPTIONAL_EXPORTS, and the __all__ export lists) with Scientific Python's `lazy_loader.attach_stub`. `deeplabcut/__init__.pyi` is now the single declarative source of truth for the public API: static tools read it for discoverability and signatures, while lazy_loader parses it at runtime to install `__getattr__`, `__dir__`, and `__all__`, importing each implementation module only when its attribute is first accessed. This keeps the flat API intact and avoids eagerly importing heavy or optional modules such as `pose_estimation_pytorch`, `gui`, and `pose_tracking_pytorch` during `import deeplabcut`. - add `lazy_loader` as a runtime dependency, - add `__init__.pyi` and `py.typed` - add tests for the new loading behavior.
…timation_pytorch.config
deeplabcut.api is the canonical entrypoint and its modules are cheap. Eager re-exports make deeplabcut.api.<name> an ordinary attribute that type checkers resolve without a stub.
…bcut.utils Currently deplabcut.utils is advertised as public entrypoint (e.g. see deeblabcut.__all__). We might want to drop this as API entrypoint in a separate PR
…azy attributes. The error handling was introduced to provide actionable messages when a specific (extra) dependency was missing, but: - torch is not an extra dependency, so should be installed and not have exceptional error messaging - the gui components are normally never imported from external scripts, only launched. raw ModuleNotFound errors + a stack trace are clear enough and don't warrant any special casing.
it was exported but missing from `__all__`. This commit ensures parity with the state before lazy loading was introduced.
`DEBUG` was missing from __all__, but included in the exports, but now it is treated like any other exported symbol
Also: do not skip the subprocess resolve all API for non-GUI modules. (i.e. only skip GUI-requiring modules if not installed)
`deeplabcut/utils/__init__.py` previously star-imported eight submodules, so many symbols were reachable directly as `from deeplabcut.utils import X`. Replacing that with lazy_loader.attach dropped them all. This commit restores the symbols manually, resolving lazily. Third party dependencies and accidental leaks are not restored (numpy, os, logger, …).
c052876 to
62f5b33
Compare
This branch has not been deployed
2 participants
Summary
Replace DeepLabCut's hand-maintained lazy export maps (_API_EXPORTS_MAP, _OPTIONAL_EXPORTS, and the all export lists) with Scientific Python's
lazy_loader.attach_stub.deeplabcut/__init__.pyiis now the source of truth for public API. Type checkers can use use it, and at runtimelazy_loaderparses it to install__getattr__,__dir__, and__all__, as well.Each implementation module is only imported when its attribute is first accessed. This avoids eagerly importing heavy or optional modules such as
pose_estimation_pytorch,gui, andpose_tracking_pytorchduringimport deeplabcut.Note that this shifts the import-time drastically to lower-level heavy modules
Changes
lazy_loaderas a runtime dependency,__init__.pyiandpy.typedDEBUGconstant to it's own moduleModuleNotFoundErrorinstead ofAttributeErrorwhich is also propagated throughhassattrNote
For future reference: this PR is scoped to change only the import strategy while fixing some big issues like circular imports. During the work, I found that within the package a lot of import hygiene can still be applied at lower levels. For instance: many modules import heavy dependencies in core, such as pandas, numpy or numba. Fixing these would yield another big win.