TST: Add direct tests for ks_2samp Cython extension in stats._stats#19378
TST: Add direct tests for ks_2samp Cython extension in stats._stats#19378dhruv1955 wants to merge 2 commits intoastropy:mainfrom
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
I don't think anyone asked for this as you did not link issue. |
|
Thanks @pllim for the feedback. I'll open issues for these test additions first and link them before reopening. The motivation is the GSoC 2026 project on hardening astropy's core stability, which specifically calls for a low-level test suite for C/Cython extensions independent of the public API. |
Huh. Has GSoC started? Who is your mentor? |
GSoC 2026 applications are open until April 8th. I'm in the contributor application phase — not yet accepted. The project is listed at https://openastronomy.org/gsoc/gsoc2026/#/apply with neutrinoceros, astrofrog, and nstarkman as mentors. I'm contributing to astropy to strengthen my application. |
|
Ah, gotcha. In that case, please clarify your motivation in your future PR descriptions and ping a mentor to review. Thanks for your interest. |
|
Also, I think any maintainer can re-open this PR if they find it valuable and wish to proceed. FYI. |
|
Indeed this is in line with our GSoC proposal. I'll re-open and review now. |
bcf89b5 to
680046c
Compare
|
Thanks @neutrinoceros Addressed all feedback and squashed to a single commit:
|
a455676 to
b2b3bf5
Compare
|
Thanks @neutrinoceros Replaced the manual |
neutrinoceros
left a comment
There was a problem hiding this comment.
Now comes the part where I learn about the algorithm in question (ks_2samp).
These tests look like a good first step but they're not strict enough, I think.
One general observation is that it might make more sense to use pytest.approx in all of them, with explicit tolerances as low as possible.
b2b3bf5 to
37ff503
Compare
|
Thanks @neutrinoceros Replaced the scipy comparisons with analytically computed expected values and |
|
In general we use assert_allclose from numpy rather than pytest.approx, so might be worth switching to that for consistency? |
37ff503 to
677dedf
Compare
|
Thanks @astrofrog, Switched to |
|
Hi @neutrinoceros - when you get a chance, would appreciate a look at the proposal. Happy to address any feedback before the deadline. |
Summary
ks_2sampinastropy/stats/_stats.pyxis a Cython implementation of the two-sample KS statistic used internally byastropy.stats, but it currently has no tests that call it directly.This PR adds a dedicated test file that exercises the Cython function independently of the public Python API.
What’s Tested
0.01.0int32,uint8) work correctly0.0and1.0