Reimplement the FITS parser natively, without kerchunk - #1090
Draft
TomNicholas wants to merge 1 commit into
Draft
Conversation
Parse FITS with astropy directly and build the ManifestStore from the HDU offsets it reports, rather than routing through kerchunk's reference dict. The fits extra now installs only astropy, and the parser reads through the ObjectStoreRegistry instead of fsspec, so reader_options is gone. Expose every data HDU rather than only the first, and name each HDU's axes after that HDU: separate HDUs routinely disagree about the length of a given axis, which a shared y/x cannot express. ASCII tables previously could not be read at all, since nothing was registered under the codec name kerchunk emitted for them. Add a real zarr v3 codec, and locate columns by TBCOL rather than assuming they tile the row with a separator between them, which is not how the format works. Parsing also has to allow the Fortran D exponents and blank fields the format uses. Fix BSCALE being passed straight to FixedScaleOffset, which divides rather than multiplies on decode, so scaled images came back scaled by 1/BSCALE. Map an integer image's BLANK keyword to a _FillValue attribute, scaled alongside the data when BSCALE or BZERO is present. Refuse binary tables. Their columns form a big-endian structured dtype, and the zarr v3 struct data type cannot record a field's byte order, so the values would read back byte-swapped once the metadata has been serialized. Returning nothing is better than returning wrong numbers. The Hubble test file turned out to contain an ASCII table that was being silently skipped, and to live in us-east-1 rather than us-west-2 -- the wrong region went unnoticed while kerchunk did the reading through fsspec and the registered store was never used.
TomNicholas
temporarily deployed
to
test-release
August 17, 2026 12:26 — with
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1090 +/- ##
==========================================
+ Coverage 89.67% 90.04% +0.37%
==========================================
Files 41 41
Lines 2682 2803 +121
==========================================
+ Hits 2405 2524 +119
- Misses 277 279 +2
🚀 New features to boost your workflow:
|
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.
Parses FITS with astropy directly and builds the
ManifestStorefrom the HDU offsets it reports, instead of routing through kerchunk's reference dict and translating it back. Thefitsextra now installs onlyastropy.Follows the same pattern as the native netCDF3 parser: read the format's own structure, build
ManifestArrays from it, skip the round-trip through kerchunk refs entirely.Coverage went up
ASCII tables (
TABLEHDUs) previously could not be read at all —get_codec_class("numcodecs.FITSAscii")raisedKeyError, so they failed at metadata construction. This adds a real zarr v3FITSAsciiTableCodec. The Hubble file used by the existing test turned out to contain an ASCII table that kerchunk was silently skipping; all six of its columns now match astropy exactly.Three correctness bugs found along the way:
BSCALEwas inverted. It was passed straight toFixedScaleOffset, which divides on decode, so every scaled image came back scaled by1/BSCALE.TBCOLplaces them, and the real Hubble table has gaps (offsets[0, 16, 32, 48, 64, 92], itemsize 104). Parsing also has to allow the FortranDexponents the format uses, which numpy cannot parse; without that the real file fails outright.BLANKwas ignored, despitecustom_parsers.mddocumenting that FITS parsers should map it. It now becomes a_FillValue, scaled alongside the data whenBSCALE/BZEROare present.Behaviour changes
SCI_y,SCI_x) rather than a sharedy/x. The shared names only worked because kerchunk emitted a single extension; with several HDUs they collide, since HDUs routinely disagree about the length of a given axis.reader_optionsis gone. The parser reads through theObjectStoreRegistryrather than fsspec, so fsspecstorage_optionsno longer apply — configure credentials on the store you register.Binary tables now raise
Their columns form a big-endian structured dtype, and the zarr v3
structdata type has no way to record a field's byte order:[('flux', '>f4')]serializes to plainfloat32and reparses little-endian, and zarr's bytes codec does not applyendianto a structured dtype anyway. So any virtual reference to big-endian record data reads back byte-swapped once the metadata has been throughzarr.json.This is pre-existing and not specific to FITS — it reproduces with a hand-built
ManifestArrayand no FITS involved. The parser previously accepted these HDUs and returned wrong numbers; it now refuses them, andskip_variablesexcludes them. Worth fixing properly at the zarr level, but that is a separate change with a much wider blast radius.Testing
Local fixtures cover images, cubes, scaled images, ASCII tables,
BLANKmasking, per-HDU axis naming, duplicateEXTNAMEdisambiguation, and the binary-table refusal, so the parser is no longer only exercised by a network test. The two network tests validate against the real Hubble file, including the previously-skipped ASCII table.The existing Hubble test passed
region="us-west-2", but the bucket isus-east-1. It went unnoticed because kerchunk read via fsspec and the registered store was never actually used; now that the parser reads through it, the region has to be right.