Read Directly From Cloud Functionality #104
Conversation
* add to utils init * refactor in io.py lines.py regions2d.py
* rename check_files to check_file_extension_existence and do necessary refactoring in other files * remove part where we check for list: we are checking for only that of .evr and .evl and nothing else (this was initially added here because I was checking for both .zarr and .nc files in the same object, but that has now been removed) * change to just check for file: no longer checking for directory, file must be of single evl or evr * used re instead of ends with for file extension checking * added description for check_file_extension_existence
* functionality to read from evl and evr files from the cloud * no tests yet and no information regarding any S3 buckets and such * will put this on hold and come back to it another time
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## main #104 +/- ##
==========================================
- Coverage 75.28% 71.24% -4.05%
==========================================
Files 13 13
Lines 433 466 +33
==========================================
+ Hits 326 332 +6
- Misses 107 134 +27
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
* replace check_file with newer check_file_extension_existence
There was a problem hiding this comment.
I think that rather than having separate functions for reading cloud files, we can have optional input argument storage_options to the existing read_evl and read_evr functions (see this example from echopype) for the credentials, and have the input path accepting both local and cloud paths.
In terms of testing, in echopype we actually test with a docker-minio server that simulates s3 functions, so that we do not rely on external sources for testing. I think we can delay the CI testing to later and just make sure the functionalities work when we use it, and circle back when we have more time to add that testing infrastructure back in.
|
Also I think we can push this to be something added at v0.1.1 (ie. not at the first release). |
|
@leewujung Oh I just took a look at the echopype implementation and it does look a lot cleaner. After I put some last touches on a draft PR for testing, I'll be waiting for Valentina to review my 2d 3d mask PR again and within transect mask PR before I can start anything else. I could probably start working on this in the meantime. I don't think it's too much to implement, if I don't implement the docker test. |
|
Sounds good! |
Read Directly From Cloud Functionality (On Hold)
This PR addresses #69:
Note:
Currently on hold. Will wait for Wu-Jung and Valentina to come back to continue work on this.