Support rpath list in asyn put #713
Conversation
|
I couldn't really find a good place to add a test for this in fsspec, looks like this is more something that is deferred to tests in the concrete async filesystems like GCSFS? I could possibly extend https://github.com/dask/gcsfs/blob/main/gcsfs/tests/test_core.py#L460 to cover this regression |
baxen
force-pushed
the
baxen/asyn_put_lists
branch
from
36ea406 to
5c2ce89
Compare
There is currently a bug that this would have caught from fsspec/filesystem_spec#712 Will fail until fsspec/filesystem_spec#713 is available.
There is currently a bug that this would have caught from fsspec/filesystem_spec#712 Will fail until fsspec/filesystem_spec#713 is available.
There is currently a bug that this would have caught from fsspec/filesystem_spec#712 Will fail until fsspec/filesystem_spec#713 is available.
|
Is this an issue for |
|
Nope no issue with _get - this was coming just from the code that was handling making directories. And I think https implementation has a good test of this already in test_multi_download I believe you saw it, but the test in fsspec/gcsfs#401 covers it indirectly at least |
martindurant
left a comment
martindurant
left a comment
There was a problem hiding this comment.
All looks good and function. Just the one question (see below).
I agree now that this will only get tested in gcsfs, but that's OK. Note that gcsfs tests run in CI here, so a test there ( fsspec/gcsfs#401 ) will show up here too after merging.
fsspec/asyn.py
Outdated
| rpaths = other_paths(lpaths, rpath) | ||
|
|
||
| rdirs = [r for l, r in zip(lpaths, rpaths) if os.path.isdir(l)] | ||
| file_pairs = [(l, r) for l, r in zip(lpaths, rpaths) if not os.path.isdir(l)] |
There was a problem hiding this comment.
Could we possibly avoid calling isdir on every file twice?
There was a problem hiding this comment.
This is now called just once per file
baxen
force-pushed
the
baxen/asyn_put_lists
branch
from
5c2ce89 to
12684e9
Compare
There is currently a bug that this would have caught from fsspec/filesystem_spec#712 Will fail until fsspec/filesystem_spec#713 is available.
There is currently a bug that this would have caught from fsspec/filesystem_spec#712 Will fail until fsspec/filesystem_spec#713 is available.
2 participants
Fixes #712