Skip to content

TST: Add direct tests for _sigma_clip_fast C extension#19377

Open
dhruv1955 wants to merge 1 commit intoastropy:mainfrom
dhruv1955:tst/fast-sigma-clip-c-extension
Open

TST: Add direct tests for _sigma_clip_fast C extension#19377
dhruv1955 wants to merge 1 commit intoastropy:mainfrom
dhruv1955:tst/fast-sigma-clip-c-extension

Conversation

@dhruv1955
Copy link
Copy Markdown
Contributor

Summary

Add a dedicated test file for the _sigma_clip_fast C extension function, testing it directly and independently of the public SigmaClip API.

Motivation

Astropy's test suite currently only tests _sigma_clip_fast indirectly through SigmaClip. These tests exercise the C extension directly, making them suitable for a future low-level test layer independent of the Python API.

Tests added

  • test_basic_returns_finite_bounds - clean input returns finite bounds
  • test_median_vs_mean_differ - use_median flag changes bounds
  • test_pre_masked_outlier_excluded - pre-masked values excluded from bound calculation
  • test_mad_std_vs_std_differ - use_mad_std flag changes bounds
  • test_max_iter_zero - max_iter=0 returns finite bounds
  • test_single_element - single element returns equal bounds
  • test_all_masked_returns_nan - all-masked input returns NaN bounds

@dhruv1955 dhruv1955 requested a review from larrybradley as a code owner March 7, 2026 19:09
@github-actions github-actions bot added the stats label Mar 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 7, 2026

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 pllim added this to the v8.0.0 milestone Mar 9, 2026
@pllim
Copy link
Copy Markdown
Member

pllim commented Mar 9, 2026

Same sentiment as #19378 (comment)

@pllim pllim closed this Mar 9, 2026
@dhruv1955
Copy link
Copy Markdown
Contributor Author

cc @neutrinoceros - same motivation as #19378 which you reopened. Happy to update this one too if it's worth proceeding.

@dhruv1955 dhruv1955 force-pushed the tst/fast-sigma-clip-c-extension branch 2 times, most recently from 9e380e9 to eb3f48d Compare March 10, 2026 16:34
@dhruv1955
Copy link
Copy Markdown
Contributor Author

Thanks @neutrinoceros Added a module-level docstring explaining the full function signature and each parameter. Also removed the changelog fragment. Squashed to a single commit.

@dhruv1955 dhruv1955 force-pushed the tst/fast-sigma-clip-c-extension branch from eb3f48d to 26cda44 Compare March 10, 2026 16:42
@dhruv1955 dhruv1955 force-pushed the tst/fast-sigma-clip-c-extension branch from 26cda44 to 5b2b8cf Compare March 11, 2026 17:02
@dhruv1955
Copy link
Copy Markdown
Contributor Author

Thanks @neutrinoceros. I've moved the parameter documentation into the C source file's docstring astropy/stats/src/fast_sigma_clip.c next to the function definition and removed it from the test file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants