feat: harden support for any file-like (including fsspec file handles). cast urls to fsspec#292
Open
tlambert03 wants to merge 12 commits intomainfrom
Open
feat: harden support for any file-like (including fsspec file handles). cast urls to fsspec#292tlambert03 wants to merge 12 commits intomainfrom
tlambert03 wants to merge 12 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
Owner
Author
|
please let me know if this works for you. It meets all the tests I want it to meet here (see 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 |
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.
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 newstorage_options: dictparameter to ND2File, since now the user will have to be able to modify things likeclient_kwargsandconfig_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 theread_planefallback when we don't have a memmap, same as in #290what 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.