Fix fallback percentile calculation

[nomad3]

Summary

• exercise _percentile_fallback with both the ordinary NumPy coverage lane and the existing array-api-strict backend on a non-default device
• calculate fallback indexes with Array API shape, dtype, percent-scale, and device-aware operations
• support integer and list-like percentiles and keep the 100th percentile inside the valid index range
• keep the existing sort-and-order-statistic behavior without selecting a new percentile interpolation convention

Fixes #888

Validation

• failing-first strict regression: 2 failures at len(Array) before the original fix
• refreshed NumPy regression: 6 passed, with a test-only namespace proxy that omits percentile so the production fallback executes in the coverage job
• refreshed strict regression: 6 passed on the non-default device, including scalar floats and integer [0, 50, 100] endpoints
• refreshed full NumPy suite: 349 passed, 29 skipped
• the earlier full JAX suite passed with 324 passed, 31 skipped, and 7 existing xfailed
• the earlier full Dask suite passed with 326 passed and 36 skipped
• Ruff, Black, and git diff --check

Checklist

• For new contributors: Did you add yourself to the "Authors.rst" file?

For documentation changes:

• For documentation changes: Does your commit message include a "[skip ci]"?

For bugfixes:

• Did you add an entry to the "Changes.rst" file?
• Did you add a regression test?
• Does the commit message include a "Fixes #issue_number"?
• Does this PR add, rename, move or remove any existing functions or parameters?

For new functionality:

• Did you add an entry to the "Changes.rst" file?
• Did you include a meaningful docstring with Parameters, Returns and Examples?
• Does the commit message include a "Fixes #issue_number"?
• Did you include tests for the new functionality?
• Does this PR add, rename, move or remove any existing functions or parameters?

AI assistance

OpenAI Codex was used for source inspection, test design, implementation, and automated review. The contributor has completed the final review and takes responsibility for the contribution's correctness and maintenance.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.72%. Comparing base (3ea1af9) to head (2c8dadb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #957      +/-   ##
==========================================
+ Coverage   96.70%   96.72%   +0.02%     
==========================================
  Files           8        8              
  Lines        1576     1589      +13     
==========================================
+ Hits         1524     1537      +13     
  Misses         52       52              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR fixes ccdproc.core._percentile_fallback so it computes percentile indices correctly for Array API namespaces that don’t implement percentile, including device-aware behavior for array-api-strict, and adds a targeted regression test to exercise the fallback path under both NumPy-coverage and strict backends.

Changes:

• Rework fallback percentile index calculation to be percent-scale correct (/100), shape-based, dtype/device-aware, and clamp the 100th percentile to n-1.
• Add a regression test that forces the fallback path for the NumPy lane (via a namespace proxy without percentile) and runs on a non-default device for array-api-strict.
• Document the bug fix in CHANGES.rst.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
CHANGES.rst	Adds a changelog entry describing the fallback percentile fix.
ccdproc/tests/test_ccdmask.py	Adds regression coverage for _percentile_fallback across NumPy and array-api-strict (non-default device).
ccdproc/core.py	Fixes fallback percentile index computation using Array API-friendly operations and clamps the 100th percentile index.

Comments suppressed due to low confidence (1)

ccdproc/core.py:103

• Docstring typo: "implmentation" should be "implementation".

def _percentile_fallback(array, percentiles, xp=None):
    """
    Try calculating percentile using namespace, otherwise fall back to
    an implmentation that uses sort. As of the 2023 version of the array API
    there is no percentile function in the API but there is a sort function.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mwcraig]

Thanks for the fix — the approach here is right, and I appreciate the fallback finally getting real test coverage on both the numpy and array-api-strict lanes. Verifying against the pre-PR code confirmed the tests genuinely exercise the fixed path.

One change requested before this is ready:

The index clamp is one-sided. The new code clamps the high end:

indexes = xp.minimum(
    indexes,
    xp.asarray(sorted_array.shape[0] - 1, ...),
)

but there's no low-side clamp or range validation. A negative percentile produces a negative index, which wraps Python-style to the top of the sorted array. I verified: with an array of 0..99, _percentile_fallback(arr, -5) returns 95.0, where np.percentile raises ValueError. Similarly, percentiles above 100 silently clamp to the maximum instead of raising. So both out-of-range directions silently return large values, and the fallback disagrees with the namespace-percentile branch on error behavior — the same call would raise under numpy but return garbage under a fallback namespace.

The current callers hard-code 30.9/69.1 so this is latent today, but since this PR is establishing the fallback's contract, could you either:

1. validate 0 <= p <= 100 up front and raise ValueError (preferred — matches np.percentile), or
2. at minimum add the low-side clamp (xp.maximum(indexes, 0)),

and add an out-of-range case to test_percentile_fallback? That's exactly the region the current parametrization doesn't touch.

Everything else looks good — the shape[0]/÷100/dtype/device handling is exactly what strict-on-non-default-device needs, and keeping floor-rank order statistics rather than picking an interpolation convention is the right call for ccdmask's use.

This review comment was written by Claude (via Claude Code) following Matt's review of the PR; Matt has reviewed and approved its content before posting.

[mwcraig]

Thanks once again @nomad3!