[pull] main from scipy:main#833
Merged
Merged
Conversation
- Int/float values that were created with separate function calls where leaked and not null-checked. - The final dict that's returned was created with "O" (which increments the refcount) rather than "N", which was hence also leaked. Found when auditing the code for a different bug (see previous commit) with Claude Opus 4.8
The two separate goto's for handling failure made the code fragile, and it seemed hard to reason about and hence easy to reintroduce bugs like the ones just fixed in a previous commit. So instead, ensure to declare all variables at the top of the function, use `Py_CLEAR` to set them to NULL (instead of using `Py_DECREF`), and have a single `fail` goto. Drafted with help from Claude Opus 4.8
…5507) * BUG: interpolate: fix potential double-free in failure path Closes gh-25435 * BUG: interpolate: fix refcounting issues in fitpack_parcur - Int/float values that were created with separate function calls where leaked and not null-checked. - The final dict that's returned was created with "O" (which increments the refcount) rather than "N", which was hence also leaked. Found when auditing the code for a different bug (see previous commit) with Claude Opus 4.8 * MAINT: interpolate: refactor `fitpack_parcur` failure gotos The two separate goto's for handling failure made the code fragile, and it seemed hard to reason about and hence easy to reintroduce bugs like the ones just fixed in a previous commit. So instead, ensure to declare all variables at the top of the function, use `Py_CLEAR` to set them to NULL (instead of using `Py_DECREF`), and have a single `fail` goto. Drafted with help from Claude Opus 4.8 reviewed at #25507
) * BUG: interpolate: coerce non-contiguous input in surfit wrappers The C bindings _fitpack.surfit_lsq and surfit_smth read the x/y/z/w buffers assuming C-contiguity, but the Python wrappers used np.asarray, which is a no-op on an already-float64 strided view such as a column arr[:, 0] of an (N, 2) array. The view reached C unchanged and was misread as interleaved coordinates, silently producing a wrong or singular fit from LSQBivariateSpline and SmoothBivariateSpline since the FITPACK C port. Coerce the buffers with np.ascontiguousarray in both wrappers, matching the existing _regrid_smth_spher pattern, and add regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * TST/MAINT: interpolate: address review on surfit contiguity fix Use np.asarray(copy=True, order="C") for the buffers the C routine writes in place or that it may write through (w/tx/ty in surfit_lsq, w in surfit_smth), and coerce tt/tp in the sphere LSQ wrapper for the same reason; keep np.ascontiguousarray for the read-only x/y/z. Replace the per-class tests with one parametrized sweep over all six public *BivariateSpline classes. Every array argument is passed as a strided, NaN-interleaved view (a single adversarial non-contiguous layout covers any non-C-contiguous input), compared against the C-contiguous result. SmoothBivariateSpline is affected too, not only LSQBivariateSpline; the four gridded/sphere classes are covered as guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * MAINT: interpolate: drop redundant order="C" from 1-D copies w/tx/ty (surfit_lsq), w (surfit_smth) and tt/tp (spherfit_lsq) are all 1-D, where np.asarray(copy=True) already returns a C-contiguous array; order="C" only affects ndim >= 2. Keep copy=True, drop the no-op. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> reviwed at #25484
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )