from_parquet: use virtual filesystem to preserve partition information when using cache #745
Conversation
Deploying datachain-documentation with
|
| Latest commit: |
097cac3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://39257ee1.datachain-documentation.pages.dev |
| Branch Preview URL: | https://from-parquet-referencefs.datachain-documentation.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #745 +/- ##
==========================================
+ Coverage 87.47% 87.51% +0.04%
==========================================
Files 114 114
Lines 10941 10945 +4
Branches 1504 1501 -3
==========================================
+ Hits 9571 9579 +8
+ Misses 992 991 -1
+ Partials 378 375 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
7b10173 to
17ef8fc
Compare
skshetry
changed the title
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
17ef8fc to
785dfab
Compare
|
@skshetry so, which PR is better to review? :) |
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
785dfab to
a138a47
Compare
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
a138a47 to
3a7e7d2
Compare
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
3a7e7d2 to
f87f0a8
Compare
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
f87f0a8 to
43580ed
Compare
I have closed the other PR as I think using a virtual fs is a much cleaner and less fragile solution. |
| @@ -190,18 +225,6 @@ def arrow_type_mapper(col_type: pa.DataType, column: str = "") -> type: # noqa: | |||
| raise TypeError(f"{col_type!r} datatypes not supported, column: {column}") | |||
|
|
|||
|
|
|||
| def _nrows_file(file: File, nrows: int) -> str: | |||
There was a problem hiding this comment.
I have removed this hack. If nrows is a significant number, this is likely to be slower.
We now iterate up to self.nrows with the pyarrow dataset.
I tried running json-csv-reader example, and the new method is slightly faster for small amount of rows.
| # disable prefetch if nrows is set | ||
| settings = {"prefetch": 0} if nrows else {} |
There was a problem hiding this comment.
disabled prefetch when nrows is set. Prefetching a large file is going to be counter-productive when you only want to read some rows.
There was a problem hiding this comment.
should we just let users disable it though?
There was a problem hiding this comment.
The fact that we are pre-downloading/pre-fetching is not going to be obvious to the users. So, finding the right settings to disable/enable them is going to be difficult.
Similarly, the default settings with prefetch is going to be suboptimal when you only want to read a few rows. In fact, I'd have to adjust example tests to disable prefetch as well, as we use laion dataset to read just a couple of rows.
That said, I do not have any strong opinion, though — please let me know what you think.
| elif nrows: | ||
| nrows += 1 |
There was a problem hiding this comment.
This was a workaround to make nrows works if the data contains headers. This is no longer needed as we count the actual rows internally.
| @@ -55,57 +67,80 @@ def __init__( | |||
| def process(self, file: File): | |||
There was a problem hiding this comment.
I have also split up this function into 2-3 smaller functions for readability.
Also did:
* minor refactor,
* removes `nrows` _hack_ and,
* disables prefetching when `nrows` is set, so that we don't download
the whole dataset.
skshetry
force-pushed
the
from-parquet-referencefs
branch
from
43580ed to
097cac3
Compare
Alternative proposal to #744, that uses a
ReferenceFileSystem(which acts as a pointer) to preserve partitioned information.