WIP: Standalone C-level tests for 1D convolution (GSoC 2026) by toastmt · Pull Request #19452 · astropy/astropy

toastmt · 2026-03-17T14:28:34Z

Description

This Draft PR serves as a proof-of-concept accompanying my GSoC 2026 proposal: Hardening Astropy’s Core Stability.

I have implemented a standalone C testing file (test_convolve_1d.c) that directly interfaces with the convolve1d_c function in astropy/convolution/src/convolve.c.

What this code does:

Bypassing the Cython wrapper (_convolve.pyx), this test file directly validates the memory handling and mathematical edge cases of the C-core. It currently tests:

Standard array convolution accuracy.

NaN interpolation handling.

The bot=0 fallback behavior when encountering arrays comprised entirely of NaNs.

Because this is a standalone executable, it can be compiled and run directly from the src directory without building the entire Python package.

I am looking forward to any feedback on this approach!

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2026-03-17T14:29:00Z

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.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

pllim · 2026-03-17T14:56:11Z

How do you run it in CI? How do you ask user to run it?

toastmt · 2026-03-17T15:07:43Z

How do you run it in CI? How do you ask user to run it?

Hi @pllim,
Because this draft PR is currently a proof-of-concept for my GSoC 2026 proposal, this specific file is completely standalone and not yet integrated into the build system.
A developer must compile and run it manually from the "src" directory:
"gcc test_convolve_1d.c convolve.c -o test_convolve -lm"
"./test_convolve"

For the actual GSoC project, the goal will be to automate this.

pllim · 2026-03-17T15:09:52Z

I don't think this idea is sustainable. astropy is a Python package first. It is enough to test it via the Python interface.

toastmt · 2026-03-17T15:24:17Z

I don't think this idea is sustainable. astropy is a Python package first. It is enough to test it via the Python interface.

This is in response to the methodology outlined in the official GSoC 2026 project idea, "Hardening astropy’s core stability," which proposes creating a fundamental test suite for the low-level C layer, in order to make it more independent from the existing astropy tests.

(https://openastronomy.org/gsoc/gsoc2026/#/projects?project=hardening_astropy's_core_stability)

I am happy to adjust the technical approach of my proposal to align with whatever consensus the maintainer team reaches regarding the testing infrastructure.

pllim · 2026-03-17T15:28:04Z

cc @neutrinoceros @nstarman @astrofrog

neutrinoceros · 2026-03-23T11:24:12Z

Sorry for being so late. I think this approach is absolutely valid and desirable.

astropy is a Python package first. It is enough to test it via the Python interface.

to restate the context: this GSoC proposal is a subset of a larger proposal to make astropy itself pure Python, but this means moving the compiled components to a different package, which is only reasonable if they have sufficient test coverage that they can be decoupled from the Python interface.

So, before pushing this, @toastmt, how would you propose to integrate this to our automated test runners ?

toastmt · 2026-03-23T18:08:58Z

Hi @neutrinoceros. If we don't want to write a Cython wrapper for testing (in order to keep the C tests decoupled from Python) we could add a compile step directly in the CI workflow to compile the C testing suite into standalone executables and then have python evaluate the tests via subprocess. That way the C testing suite would be prepared for a future decoupling of the C-core or a migration to Meson.

neutrinoceros · 2026-03-24T09:05:05Z

That seems reasonable to me. Feel free to give it a try.

pllim · 2026-03-24T14:16:13Z

C testing suite into standalone executables

This will annoy me and probably other devs because it polutes the bin space for no reason. I stress again #19452 (comment)

neutrinoceros · 2026-03-24T14:20:47Z

Okay let's reframe this as an experiment then;
I think he potential for annoyance on maintainers would instantly get much lower if the APE was accepted and implemented, so maybe we don't want to merge this PR as long as the code lives in the main repo, but I'm still interested to know how it'd actually look if we were to adopt this strategy (among others) when/if they are decoupled.

pllim · 2026-03-24T14:29:43Z

potential for annoyance on maintainers would instantly get much lower

I doubt it. I maintain a C pipeline elsewhere.

toastmt · 2026-03-27T02:21:10Z

I've been giving this a try. I've implemented a pytest script that evaluates the C tests via subprocess. I created a new job in the YAML workflow file to compile the C test binary and then run the pytest file. I also inserted a mock NumPy header creation step in the CI workflow to decouple the C compilation from the Python environment.

The remaining hurdle is fully isolating pytest from astropy's pyproject.toml and conftest.py, which pull in the full package. Open to suggestions on the cleanest approach here.

neutrinoceros · 2026-03-27T15:56:42Z

.github/workflows/ci_workflows.yml


+  test_c_extensions:
+    # Standalone tests for the C math extensions of Astropy
+    # Create a mock ndarrayobject header to decouple C logic from Python/NumPy 


I would expect test binaries to not need numpy at all. The fact that you need a mock header to make it work makes me doubt the value of the approach; it's not decoupled if you need a mock.

I agree that a mock header isn't true decoupling. The issue is that the mock header is providing the type definitions that convolve.c needs. The alternative would be refactoring convolve.c to use standard C types like double instead of npy_float64, but that approach touches core C files across the repository. Is there a different approach you'd recommend for achieving true decoupling without modifying the core?

I think it makes sense to slightly edit the C modules in this way (replacing a type alias with its primitive definition): the benefit of decloupling outweights the cost of refactoring, in my opinion. That is, I think it does, but maybe I'm not seeing the complete picture yet. Could you explain the benefit(s) of using npy_float64 instead of double ?

npy_float64 is effectively just a typedef for double. However, npy_float64 guarantees 64-bit precision across all systems and platforms, while double doesn't. We can use a _Static_assert in the header to guarantee that with a double too. I could try to refactor the convolve.h header slightly to decouple the C file from NumPy.

toastmt added 3 commits March 17, 2026 15:57

File for initial 1D convolution tests

93fcd8c

Added 1d NaN array test

664fe82

Added bot=0 fallback test

00b3538

github-actions bot added the convolution label Mar 17, 2026

pllim added this to the v8.0.0 milestone Mar 17, 2026

pllim added testing no-changelog-entry-needed labels Mar 17, 2026

toastmt added 7 commits March 26, 2026 01:24

TST: Move convolution C test file to dedicated C tests directory

d3d4e8f

TST: Refine 1d convolution tests with stdout, stderr logging

4466a3b

TST: Add CI job to compile and verify standalone C tests

5fc325c

TST: Fix CI job dependencies and code style

1a5ed2e

TST: Fix pytest isolation from astropy conftest / Fix code style

57bbc29

Merge remote-tracking branch 'upstream/main' into c-convolve-testing

9f5cb17

TST: Fix pytest config isolation and clang-format code style

810ae73

neutrinoceros reviewed Mar 27, 2026

View reviewed changes

Uh oh!

Conversation

toastmt commented Mar 17, 2026

Description

Uh oh!

github-actions bot commented Mar 17, 2026

Uh oh!

pllim commented Mar 17, 2026

Uh oh!

toastmt commented Mar 17, 2026

Uh oh!

pllim commented Mar 17, 2026

Uh oh!

toastmt commented Mar 17, 2026

Uh oh!

pllim commented Mar 17, 2026

Uh oh!

neutrinoceros commented Mar 23, 2026

Uh oh!

toastmt commented Mar 23, 2026

Uh oh!

neutrinoceros commented Mar 24, 2026

Uh oh!

pllim commented Mar 24, 2026

Uh oh!

neutrinoceros commented Mar 24, 2026

Uh oh!

pllim commented Mar 24, 2026

Uh oh!

toastmt commented Mar 27, 2026

Uh oh!

neutrinoceros Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

toastmt Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

toastmt Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants