Skip to content

Comments

feat: harden support for any file-like (including fsspec file handles). cast urls to fsspec#292

Open
tlambert03 wants to merge 12 commits intomainfrom
remote
Open

feat: harden support for any file-like (including fsspec file handles). cast urls to fsspec#292
tlambert03 wants to merge 12 commits intomainfrom
remote

Conversation

@tlambert03
Copy link
Owner

closes #290

This implements the full test suite I needed to see over in #290, and exposed a number of additional issues that needed to be addressed. Ultimately, nd2 was already very close to being able to support any arbitrary file-like, including any fsspec abstract file handle ... which is an even more general case of casting the user's s3:// string for them. So this implements that full support, but also casts strings for the user. We add a new storage_options: dict parameter to ND2File, since now the user will have to be able to modify things like client_kwargs and config_kwargs). Most of the diff here is related to typing (hardening the assumptions about accepting and Read/Seek Binary stream). and then there is also the read_plane fallback when we don't have a memmap, same as in #290

what this doesn't have is any additional public api around parallel_read_plane. If I'm not mistaken, all of that could just as easily be accomplished by the end user by creating their own executor and batch calling the existing read_plane in a loop. Since it's still likely to be much less common usage, I'd prefer to just ask the end-user to do their own parallelization rather than add complexity (and assumptions) for a possibly rarely used feature.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 19, 2026

Merging this PR will not alter performance

✅ 13 untouched benchmarks


Comparing remote (1dd2e46) with main (cfc95b6)

Open in CodSpeed

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

❌ Patch coverage is 89.51613% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.47%. Comparing base (cfc95b6) to head (1dd2e46).

Files with missing lines Patch % Lines
src/nd2/_readers/protocol.py 83.33% 8 Missing ⚠️
src/nd2/_parse/_chunk_decode.py 89.47% 2 Missing ⚠️
src/nd2/_readers/_modern/modern_reader.py 88.88% 2 Missing ⚠️
src/nd2/_nd2file.py 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
- Coverage   93.65%   93.47%   -0.18%     
==========================================
  Files          22       22              
  Lines        2630     2697      +67     
==========================================
+ Hits         2463     2521      +58     
- Misses        167      176       +9     

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

@tlambert03
Copy link
Owner Author

@derekthirstrup

please let me know if this works for you. It meets all the tests I want it to meet here (see test_s3.py).

as mentioned above, I haven't extended the public api with read_plane_parallel, but that should be easy enough for you to do directly wherever you are using read_plane with your own executor

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