| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
@natsukium, thanks a lot for your recent valuable contributions. These are definitely improving the codebase!
Regarding this PR:
I wouldn't be in favor of removing the upper bound just yet. Instead of directly jumping to full numpy 2 support I would prefer to accept all the improvements in this PR as a preparatory step for forward compatibility.
The reason I foresee issues:
If we can leave the upperbound as is for the current PR, I strongly approve of your changes!
Sorry, something went wrong.
|
By the way, would you mind if I push directly to this branch? I might add one or two commits if I spot any remaining cases. |
Sorry, something went wrong.
There was a problem hiding this comment.
This PR updates DeepLabCut to support NumPy 2.x by lifting the core dependency cap to <3 and applying targeted compatibility fixes for APIs/behaviors that changed or were removed in NumPy 2.
Changes:
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file| File | Description |
|---|---|
| tests/test_inferenceutils.py | Makes the multi-animal assembly test order-independent to accommodate NumPy 2 sort tie-breaking changes. |
| pyproject.toml | Lifts NumPy upper cap to <3 and raises lower bound to >=1.22.4. |
| deeplabcut/refine_training_dataset/stitch.py | Replaces SciPy interpolative SVD usage with NumPy SVD for rank estimation (compatibility-related). |
| deeplabcut/pose_estimation_tensorflow/core/predict_multianimal.py | Uses np.trapezoid when available to avoid NumPy 2 deprecations, with fallback to np.trapz. |
| deeplabcut/pose_estimation_pytorch/apis/prune_paf_graph.py | Same np.trapezoid/np.trapz compatibility adjustment for AUC/separability computation. |
| deeplabcut/core/trackingutils.py | Switches np.product to np.prod for NumPy 2 compatibility. |
| deeplabcut/core/inferenceutils.py | Updates np.percentile call to use method= (NumPy 2 compatible). |
| deeplabcut/core/crossvalutils.py | Same np.trapezoid/np.trapz compatibility adjustment for AUC/separability computation. |
deeplabcut/refine_training_dataset/stitch.py:394
eigen = s**2
diff = np.abs(np.diff(eigen / eigen[0]))
return np.argmin(diff > tol)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
|
Linking related issue #3240 here (dropping albumentations) |
Sorry, something went wrong.
np.percentile's interpolation= keyword is removed in NumPy 2, and its replacement, method=, only exists from NumPy 1.22. Expressing that call with a single spelling that works across the whole supported range therefore requires a floor of at least 1.22. Raising the declared floor changes nothing that could previously be installed: pandas>=2.2, a core dependency, already requires numpy>=1.22.4 on Python 3.10, the lowest supported interpreter, so numpy 1.18.5 was never selectable.
np.trapz, np.product, and np.percentile's interpolation keyword raise AttributeError or TypeError on recent NumPy 2.x, where the core pipeline already runs. Switch to np.trapezoid (np.trapz fallback for NumPy 1), np.prod, and np.percentile(method=); all behave identically from NumPy 1.22, the lower bound, up. https://numpy.org/doc/stable/numpy_2_0_migration_guide.html#main-namespace https://numpy.org/doc/stable/reference/generated/numpy.percentile.html
Tracklet stitching estimated a Hankelet's rank from its singular values via scipy.linalg.interpolative.svd. On NumPy 2 that randomized routine raises "array must not contain infs or NaNs" on finite input, so any stitching that computes tracklet affinities fails at runtime. Only the singular values matter, so use np.linalg.svd(compute_uv=False): exact, deterministic, identical on NumPy 1.22.4 and 2. It also returns zeros for an all-zero matrix instead of raising, making the DeepLabCut#2827 guard against scipy>=1.11's all-zero ValueError redundant; it is removed. scipy is removing the backend's noncompliant randomization: scipy/scipy#18367
The test checked assembled keypoints against a stored ground truth with np.testing.assert_equal. The order of equally-scored assemblies within a frame is arbitrary and not fixed by NumPy's sorts, and NumPy 2 emits two of them swapped, so the comparison failed though every coordinate matched. Sort both point sets into a canonical order first, pinning the coordinates without constraining the ordering.
|
Thanks for the careful review! I couldn't reproduce numpy 2 problems with albumentations, filterpy or scipy. Also, would it be worth adding a non-blocking CI job that runs the suite against numpy 2? Feel free to push directly to this branch. Note that I force-pushed just now, so please pull first. |
Sorry, something went wrong.
This commit partly reverts 4b977b4 where a divide by zero guard was removed. An all-zeros array is not a realistic scenario, but we are not winning anything by removing the guard.
The function existed identically (verbatim) in crossvalutils.py`and `prune_paf_graph.py`. Now deduplicated and imported.
Replace inline `(np.trapezoid if hasattr(...) else np.trapz)` runtime checks with a `_trapz = getattr(np, "trapezoid", np.trapz)` constant in both `crossvalutils.py` and `predict_multianimal.py`. The attribute test now runs once at import time instead of on every hot-path call.
|
Regarding your recommendation: I agree that it will be worthwhile adding a non-blocking CI test for this, good suggestion! This holds also for other dependencies as well. I'm opening a separate PR for this. |
Sorry, something went wrong.
There was a problem hiding this comment.
@natsukium, thanks again! I've pushed just a few minor additions. (moving the hassatr check out of the hot path; removing a duplicate function). Let me know if you agree.
The PR looks great and I think it would be useful to merge soon!
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Much of the scientific-Python ecosystem has moved to NumPy 2, so the numpy<2 cap in the core dependencies increasingly conflicts with other packages sharing an environment. This PR does the groundwork for eventually lifting that cap while leaving the cap itself in place: following the review, it is a preparatory step for forward compatibility rather than a switch to NumPy 2.
Each fix is a separate commit, with the details and rationale in its message.
What changes
The numpy<2 upper bound is unchanged.
Why the cap stays
The imgaug concern raised in review is the blocker, and a hard one: imgaug 0.4.0 fails at import under NumPy 2.
There has been no imgaug release since 0.4.0 (June 2020), so this will not be fixed upstream, and imgaug is currently a core dependency rather than a TF extra.
Moving imgaug to the TF extras (#3240) is necessary but not sufficient. imgaug declares no numpy upper bound of its own, and neither do all the pinned TensorFlow versions — only TF 2.15–2.17 constrain numpy to <2:
So when the core cap is eventually lifted, numpy<2 should move to the TF extras alongside imgaug. Otherwise [tf-cu11], [tf-cu12] and [tf-latest] installs would pair imgaug with NumPy 2 and fail at import.
The other dependencies raised in review did not reproduce as blockers (checked on NumPy 2.4.6 / Python 3.11):
Testing
The core test suite passes identically on NumPy 1.22.4 and NumPy 2.
Running the test suite against NumPy 2 in CI as a non-blocking job would keep this from regressing while the remaining blockers are resolved. Happy to add that here or in a follow-up.