Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add merge_offset_ranges utility #780

Merged
merged 6 commits into from
Oct 4, 2021

Conversation

rjzamora
Copy link
Contributor

@rjzamora rjzamora commented Sep 29, 2021

This implements a simple merge_offset_ranges utility to support the max_gap option for the cat_ranges method added in #744. Note that this utility also recognizes a max_block= argument to help preserve parallelism for large reads. I intend to use this utility (via cat_ranges) to help improve remote-file-system performance in Dask.

Copy link
Member

@martindurant martindurant left a comment

Choose a reason for hiding this comment

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

Great to see this!

I have the one API question that I think needs to be answered.
Note that even without solving that question, the code would already be useful, but require the called to themselves call marge_offset_ranged, and then cat_ranges with the result (i.e., not actually use the max_gap arg of the latter).

fsspec/asyn.py Outdated Show resolved Hide resolved
@rjzamora rjzamora marked this pull request as ready for review October 4, 2021 13:22
@martindurant
Copy link
Member

Thank you for the changes, @rjzamora !

Can I please request that you remove the copyright and licensing blocks, since this whole project is already licensed as BSD-3.

That aside, this is good to go. I was about to say you should add the to the API docs, but maybe that can wait until we see any usage aside from our own.

fsspec/tests/test_utils.py Outdated Show resolved Hide resolved
fsspec/utils.py Outdated Show resolved Hide resolved
@rjzamora
Copy link
Contributor Author

rjzamora commented Oct 4, 2021

Can I please request that you remove the copyright and licensing blocks, since this whole project is already licensed as BSD-3.

Sounds good. Thanks for the review @martindurant !

@martindurant martindurant merged commit 512d21b into fsspec:master Oct 4, 2021
@rjzamora rjzamora deleted the implement-max-gap branch October 4, 2021 16:39
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.

2 participants