search

uber / petastorm

Petastorm library enables single machine or distributed training and evaluation of deep learning models from datasets in Apache Parquet format. It supports ML frameworks such as Tensorflow, Pytorch, and PySpark and can be used from pure Python code.
Apache License 2.0
1.78k stars 285 forks source link

Fix GCSFS walk() method #561

Open megaserg opened 4 years ago

megaserg commented 4 years ago

Resolves #558

codecov[bot] commented 4 years ago

Codecov Report

Attention: Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.

Project coverage is 86.13%. Comparing base (589d40c) to head (b87919b). Report is 124 commits behind head on master.

Files Patch % Lines
petastorm/gcsfs_helpers/gcsfs_wrapper.py 0.00% 12 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #561 +/- ## ========================================== + Coverage 86.08% 86.13% +0.05% ========================================== Files 87 87 Lines 4972 4969 -3 Branches 793 792 -1 ========================================== Hits 4280 4280 + Misses 563 560 -3 Partials 129 129 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

megaserg commented 4 years ago

@selitvin @aleks-djuric Not sure what does the complaint from codecov/patch mean. Would you like to review?

selitvin commented 4 years ago

@aleks-djuric ,If you are familiar with gcs nuances, perhaps you could help review this PR?

alekswithakayy commented 4 years ago

I fixed this by just doing obj_path.strip('/'). Not familiar with how this process works. Should I create a new pull request?

selitvin commented 4 years ago

Either @megaserg incorporates the fix you propose (I think it's preferable since we'll keep only one PR for the issue), or create a separate PR and we can review and land that.

@megaserg : we don't have tests for GCSFS and not sure if we can set them up easily. We will be able to land the PR regardless.

selitvin commented 4 years ago

@megaserg, as a last ask, since we don't have a proper unittest setup, can you please paste an example of how the walk function output looks like to the comment of this PR for a sanity check?