add support for arbritary remote storages #24
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24 +/- ##
==========================================
- Coverage 83.01% 82.66% -0.36%
==========================================
Files 6 6
Lines 689 721 +32
==========================================
+ Hits 572 596 +24
- Misses 117 125 +8
Continue to review full report at Codecov.
|
sparsity/io.py
Outdated
| } | ||
|
|
||
| try: | ||
| import s3fs |
There was a problem hiding this comment.
There's already s3fs import above. Wrapping this one in try doesn't seem to make sense.
sparsity/io.py
Outdated
| if not filename.startswith('s3://'): | ||
|
|
||
| protocol = urlparse(filename).scheme or 'file' | ||
| if protocol == 'file': | ||
| fp = open(filename, 'wb') |
There was a problem hiding this comment.
Do we leave the file open here? O.o
sparsity/io.py
Outdated
| def _save_remote(buffer, filename, block_size=None, storage_options=None): | ||
| if storage_options is None: | ||
| storage_options = {} | ||
| protocol = urlparse(filename).scheme or 'file' |
There was a problem hiding this comment.
scheme shouldn't be 'file' here. I think leaving this as an option is misleading.
sparsity/io.py
Outdated
| if storage_options is None: | ||
| storage_options = {} | ||
| protocol = urlparse(filename).scheme or 'file' | ||
| open_f = FILE_SYSTEMS[protocol](**storage_options).open | ||
| fp = open_f(filename, 'rb') | ||
|
|
||
| loader = np.load(fp) |
There was a problem hiding this comment.
Same here - we're not closing the file.
There was a problem hiding this comment.
Hm I think numpy closes it will check if that's true for sure though
There was a problem hiding this comment.
Numpy documentation doesn't say that they close the file.
defaults back to minimal implementation.
|
@michcio1234 Ok to merge? |
|
Argh closed this accidentally |
Mainly to support gcsfs