Skip to content

fix: add nsamples to recognised sigproc header keys#54

Closed
themavik wants to merge 1 commit into
FRBs:mainfrom
themavik:fix/nsamples-keyerror-53
Closed

fix: add nsamples to recognised sigproc header keys#54
themavik wants to merge 1 commit into
FRBs:mainfrom
themavik:fix/nsamples-keyerror-53

Conversation

@themavik

@themavik themavik commented Feb 11, 2026

Copy link
Copy Markdown

Summary

Fixes #53

Filterbank files that include nsamples in their binary header cause parse_header() to crash with KeyError: 'nsamples' because the key is not in the header_keys dictionary.

Root Cause

In sigpyproc/io/sigproc.py, header_keys defines the set of recognised sigproc header keywords and their struct format codes. The nsamples keyword is a valid sigproc header field (unsigned 32-bit integer), but it was missing from the dictionary.

When parse_header() reads a header key from the file and looks up its format:

key_fmt = header_keys[key]  # KeyError: 'nsamples'

it crashes because nsamples is not a recognised key.

Fix

Added "nsamples": "I" to the header_keys dictionary. This allows the parser to correctly read past the nsamples field in the binary header.

The computed nsamples value on lines 288-290 still overwrites whatever the file declares, preserving the existing behavior of deriving the sample count from the actual file dimensions (datalen, nbits, nchans).

Testing

  • Filterbank files without nsamples in the header: behavior unchanged
  • Filterbank files with nsamples in the header: parsed correctly instead of crashing
  • spp_header print works on files that previously triggered the crash

Some filterbank files include nsamples in their binary header. Since
nsamples was not in the header_keys dictionary, parse_header() raised
KeyError when encountering it.

Add "nsamples": "I" (unsigned 32-bit integer) to header_keys so the
parser can read past the field. The computed nsamples value on line
288 still overwrites whatever the file declares, preserving the
existing behavior of deriving it from actual file dimensions.

Fixes #53

Co-authored-by: Cursor <cursoragent@cursor.com>
@pravirkr

Copy link
Copy Markdown
Collaborator

Thanks for the PR. It required extensive refactoring because "nsamples" is a legacy keyword, and we do not want to make it rigid. Also, we need to decide at runtime which nsamples to use: metadata or file length. Solving this in #56. Closing this now.

@pravirkr pravirkr closed this Feb 22, 2026
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.

KeyError: 'nsamples'

2 participants