Skip to content

Don't scale binary table columns without TSCAL/TZERO - #50

Merged
barrettp merged 1 commit into
mainfrom
fix-int-scaling
Oct 2, 2026
Merged

barrettp merged 1 commit into
mainfrom
fix-int-scaling

Conversation

@barrettp

@barrettp barrettp commented Oct 2, 2026

Copy link
Copy Markdown
Member

Problem

When a binary table column has no TSCAL/TZERO, the defaults are 1.0f0/0.0f0 (Float32), and the readers compute field.zero .+ field.scale.*data. That promotes integer data to Float32, so Int32 values above 2^24 are silently rounded:

stored read
16777217 16777216
16777275 16777276
33554433 33554432
2000000001 2000000000

For a variable-length (PJ) column the Float32 result is converted back into the Vector{Int32}, so the type looks right but the values are wrong. Fixed-width integer columns come back as Float32. This corrupted the pointer table of XSTAR's atdb.fits (~1M of 1.2M offsets), which astropy reads exactly.

Fix

At the top of both read(::IO, ::BinaryField, ...) methods, set scale = scale && !(field.scale == 1 && field.zero == 0) so columns with no scaling pass through unchanged. Columns with an actual TSCAL/TZERO behave as before.

This changes the output type of unscaled integer columns: they are returned as stored (Int32 stays Int32) instead of Float32.

Tests

  • New regression test test/data/int32_large.fits (written with astropy; fixed 8J and variable-length PJ columns around 2^24). It fails without the change and passes with it.
  • The existing suite passes.
  • Checked on the real atdb.fits: all 1,216,791 real-array offsets match a running sum of the record counts, as astropy reads them.

The image, primary, ASCII table and random-groups readers use the same zero .+ scale.*data pattern and are not changed here.

🤖 Generated with Claude Code

Without TSCAL/TZERO the scale and zero default to 1.0f0 and 0.0f0, so reading a
column computed zero .+ scale.*data and promoted integer data to Float32. Int32
values above 2^24 were silently rounded (16777217 -> 16777216), and fixed-width
integer columns came back as Float32. This broke variable-length Int32 columns
holding large offsets, e.g. the pointer table of XSTAR's atdb.fits.

Skip scaling when the scale is the identity so the data pass through with their
stored type. Add a regression test with an astropy-written file.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@barrettp
barrettp merged commit 83b34f1 into main Oct 2, 2026
3 checks passed
@barrettp
barrettp deleted the fix-int-scaling branch October 2, 2026 20:59
@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.53%. Comparing base (870e4ee) to head (18709d2).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   46.35%   47.53%   +1.17%     
==========================================
  Files          17       17              
  Lines        2550     2552       +2     
==========================================
+ Hits         1182     1213      +31     
+ Misses       1368     1339      -29     

☔ 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant