Skip to content

Maint decoders - #724

Merged
AhmetNSimsek merged 14 commits into
mainfrom
maint_decoders
Oct 22, 2026
Merged

AhmetNSimsek merged 14 commits into
mainfrom
maint_decoders

Conversation

@AhmetNSimsek

@AhmetNSimsek AhmetNSimsek commented Aug 28, 2026 •

Copy link
Copy Markdown
Collaborator

This PR refactors retrieval decoders to support both file- and byte-based decoding.

Key changes:

  • Introduces a Decoder abstraction with FILE and BYTES input modes.
  • Allows decoders to define kwargs passed to the underlying reader.
  • Preserves meaningful file suffixes in cache filenames.
  • Moves path-capable formats such as NIfTI, GIFTI, FreeSurfer annotations, HDF5/NWB, NumPy, and tabular data to file-based decoding where appropriate. Gzip-compressed variants (e.g. .nii.gz, .csv.gz) are handled natively by their underlying readers when given a file path.
  • Removes unnecessary file → bytes → file roundtrips and enables file-backed/lazy access where supported.
  • Keeps find_suitable_decoder() as a backwards-compatible wrapper around Decoder.from_filename().
  • Fixes ZipfileConnector to use ZipfileRequest directly instead of the bespoke ZipFileLoader, which was passing raw bytes to FILE-type decoders. ZipfileRequest correctly handles both decoder modes: BYTES decoders receive in-memory bytes, while FILE decoders have the zip member extracted to the siibra cache first and receive the path.
  • Fixes RepositoryConnector.get_loader to pass the resolved Decoder directly to HttpRequest rather than wrapping it in a lambda, which also caused raw bytes to be passed to FILE-type decoders.
  • Changes ZipfileRequest member matching from suffix-based to exact name matching, consistent with how ZipfileConnector.search_files returns full member paths.

Custom func= callbacks retain the existing byte-based behaviour.

@AhmetNSimsek AhmetNSimsek self-assigned this Aug 28, 2026
@AhmetNSimsek AhmetNSimsek added the maintenance Not a bug or breaking issue. Code maintenance related. label Aug 28, 2026
@AhmetNSimsek
AhmetNSimsek marked this pull request as ready for review September 1, 2026 13:48
@AhmetNSimsek
AhmetNSimsek marked this pull request as draft September 15, 2026 11:50
Comment thread siibra/configuration/configuration.py Outdated
@AhmetNSimsek
AhmetNSimsek marked this pull request as ready for review September 16, 2026 13:15

@xgui3783 xgui3783 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have tested enough, then go for it.

My concern is commented.

Comment thread siibra/retrieval/requests.py
Comment thread siibra/retrieval/requests.py Outdated
Comment thread siibra/retrieval/requests.py Outdated
@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.56989% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.21%. Comparing base (dd0f01f) to head (3036233).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
siibra/retrieval/requests.py 84.52% 13 Missing ⚠️
siibra/retrieval/repositories.py 0.00% 5 Missing ⚠️
siibra/configuration/factory.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #724      +/-   ##
==========================================
+ Coverage   45.94%   46.21%   +0.27%     
==========================================
  Files          74       74              
  Lines        8371     8393      +22     
==========================================
+ Hits         3846     3879      +33     
+ Misses       4525     4514      -11     

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

@AhmetNSimsek
AhmetNSimsek merged commit cd64246 into main Oct 22, 2026
196 of 202 checks passed
@AhmetNSimsek
AhmetNSimsek deleted the maint_decoders branch October 22, 2026 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Not a bug or breaking issue. Code maintenance related.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants