create sub-directories when downloading files with !#523
create sub-directories when downloading files with !#523
Conversation
amarjandu
force-pushed
the
issues/amar/1594-subdirs
branch
5 times, most recently
from
d2c72a5 to
e9fe6a3
Compare
amarjandu
force-pushed
the
issues/amar/1594-subdirs
branch
from
e9fe6a3 to
fd8ca84
Compare
Codecov Report
@@ Coverage Diff @@
## master #523 +/- ##
=======================================
Coverage 85.63% 85.63%
=======================================
Files 40 40
Lines 1893 1893
=======================================
Hits 1621 1621
Misses 272 272
Continue to review full report at Codecov.
|
jessebrennan
left a comment
jessebrennan
left a comment
There was a problem hiding this comment.
Thanks for adding the test as well!
| for entry in os.scandir(path=path): | ||
| if entry.is_file(): | ||
| files.append(entry.name) | ||
| if entry.is_dir(): |
There was a problem hiding this comment.
| if entry.is_dir(): | |
| elif entry.is_dir(): |
and else: assert False at the end.
| if entry.is_file(): | ||
| files.append(entry.name) | ||
| if entry.is_dir(): | ||
| nested_files = get_uploaded_file_names(entry.path) |
There was a problem hiding this comment.
This is building an excessive number of temporary strings and lists during recursion. Think about how you can make this more efficient. Also check if there isn't built-in functionality for performing a recursive listing of files.
|
|
||
| with tempfile.TemporaryDirectory() as dest_dir: | ||
| self.client.download(bundle_uuid=bundle_output['bundle_uuid'], replica="aws", download_dir=dest_dir) | ||
| nested_downloaded_files = [file.name for file in os.scandir('{}/{}/zarr'.format(dest_dir, bundle_fqid))] |
There was a problem hiding this comment.
Do you really have to list all files to assert that two of them were downloaded? What about os.path.isfile?
5 participants
No description provided.