Skip to content

BF: Improve TRX loading when local file headers have extra bytes#103

Open
skoudoro wants to merge 1 commit intotee-ar-ex:masterfrom
skoudoro:fix-local-header
Open

BF: Improve TRX loading when local file headers have extra bytes#103
skoudoro wants to merge 1 commit intotee-ar-ex:masterfrom
skoudoro:fix-local-header

Conversation

@skoudoro
Copy link
Collaborator

@skoudoro skoudoro commented Feb 6, 2026

this fix #86.

ZIP/TRX files have two places where they store info about each file inside:

  • A "local header" right before each file's data
  • A "central directory" at the end (like a table of contents)

The old code assumed these two always match. But they don't have to! Some tools add extra bytes to local headers that aren't in the central directory.

So when loading, we calculated the wrong position to read data from. which was causing the crash on uncompressed data that @neurolabusc encounter.

Instead of guessing where the data starts, we now read the actual local header to find out exactly where each file's data begins.

Added a test that creates a ZIP with mismatched headers - fails before fix, passes after. You can also test with the data shared in the issue #86.

Note: maybe we should add @neurolabusc dataset in testdata (if you are ok, of course) to make sure we do not encounter this issue in other languages.

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.53%. Comparing base (6b4a53d) to head (a521859).

Files with missing lines Patch % Lines
trx/trx_file_memmap.py 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
+ Coverage   59.95%   60.53%   +0.58%     
==========================================
  Files          13       13              
  Lines        2462     2501      +39     
==========================================
+ Hits         1476     1514      +38     
- Misses        986      987       +1     
Flag Coverage Δ
unittests 60.53% <95.23%> (+0.58%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

… different extra fields than central directory entries which was causing a crash.
@neurolabusc
Copy link
Member

Happy for you to inlcude any data from me, but I always like minimal examples. You could always convert the simple.trk from nibabel to trx to demonstrate, as it is tiny.

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.

Issue with some uncompressed TRX files

2 participants