Restore ValueError expectation in test_generator_ccds_without_unit - #961
Merged
Conversation
PR astropy#925 changed this test to assert that on the jax backend ccds() returns a CCDData for unit-less files instead of raising ValueError. That encodes a Linux-only bug symptom as expected behavior: on macOS with identical astropy/jax versions, ccds() does raise ValueError, so the jax branch of the test fails there and is silently absorbed by the backend_xfail marker added in cf4f4b7 for the same symptom. With both in place the test can never fail on jax on any platform. Revert the test body to expect ValueError on all backends and keep the backend_xfail marker, which documents the unresolved platform-dependent jax behavior. Raising for unit-less files is the intended behavior; the Linux jax deviation stays visible as an xfail instead of being asserted as correct. Follows up on a Copilot review comment on astropy#925. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01463EbSeiTmBJevbvpMNRm8
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #961 +/- ##
=======================================
Coverage 96.63% 96.63%
=======================================
Files 8 8
Lines 1574 1574
=======================================
Hits 1521 1521
Misses 53 53 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
6 tasks
59 tasks
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to a Copilot review comment on #925 (merged), which pointed out that
test_generator_ccds_without_unitnow carries both thebackend_xfail("jax")marker (added in cf4f4b7) and an explicit jax branch asserting thatccds()returns aCCDDatafor unit-less files.Those two contradict each other. The marker's reason documents that the jax behavior is platform-dependent: on Linux CI
ccds()fails to raiseValueError, while on macOS with identical astropy/jax versions it does raise. The jax branch added in #925 asserts the Linux behavior unconditionally, so on macOS it fails and is silently absorbed by the marker as an xfail; on Linux it passes and is reported as a (non-strict) xpass. Net effect: the test can never fail on jax on any platform, and on macOS it asserts the wrong thing.This PR reverts the test body to expect
ValueErroron all backends and keeps thebackend_xfailmarker. Raising for unit-less files is the intended behavior; the unexplained Linux-only jax deviation stays visible as an xfail rather than being encoded as correct. (Copilot suggested removing the marker instead, but that would break macOS jax runs given the platform dependence.)Verified locally:
test_image_collection.pypasses under numpy and dask; under jax on macOS the reverted test xpasses (non-strict), and on Linux CI it should xfail as it did before #925.Checklist
CHANGES.rstfile? (Not needed — test-only change with no user-facing behavior.)🤖 Generated with Claude Code
https://claude.ai/code/session_01463EbSeiTmBJevbvpMNRm8