[CHORE] Populate previews only when show() or __repr__() is called #1889
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1889 +/- ##
=======================================
Coverage 85.55% 85.55%
=======================================
Files 55 55
Lines 6194 6216 +22
=======================================
+ Hits 5299 5318 +19
- Misses 895 898 +3
|
colin-ho
commented
Feb 15, 2024
| @@ -24,7 +24,7 @@ def test_decimal_parquet_roundtrip() -> None: | |||
| df.write_parquet(dirname) | |||
| df_readback = daft.read_parquet(dirname).collect() | |||
|
|
|||
| assert str(df.to_pydict()["decimal128"]) == str(df_readback.to_pydict()["decimal128"]) | |||
| assert str(df.to_pydict()["decimal128"]) == str(df_readback.to_pydict()["decimal128"]) | |||
There was a problem hiding this comment.
this was weird, if i didn't do the indent, it would fail with:
../../daft/api_annotations.py:31: in _wrap
return timed_method(*args, **kwargs)
../../daft/analytics.py:185: in tracked_method
return method(*args, **kwargs)
../../daft/dataframe/dataframe.py:1316: in to_pydict
return result.to_pydict()
../../daft/runners/partitioning.py:214: in to_pydict
merged_partition = self._get_merged_vpartition()
../../daft/runners/pyrunner.py:52: in _get_merged_vpartition
return MicroPartition.concat([part for id, part in ids_and_partitions])
@classmethod
def concat(cls, to_merge: list[MicroPartition]) -> MicroPartition:
micropartitions = []
for t in to_merge:
if not isinstance(t, MicroPartition):
raise TypeError(f"Expected a MicroPartition for concat, got {type(t)}")
micropartitions.append(t._micropartition)
> return MicroPartition._from_pymicropartition(_PyMicroPartition.concat(micropartitions))
E ValueError: DaftError::External Internal IO Error when Opening: /var/folders/nl/4tv7_psn7fq3n8sn6b4cpvw80000gn/T/tmputa6b7lc/ce958d3e-9fdc-48c3-9c8b-1917639037cd-0.parquet:
E Details:
E No such file or directory (os error 2)
```
There was a problem hiding this comment.
oh i see, .collect() creates an unloaded micropartition, so it didn't do any actual reading of the parquet data
colin-ho
commented
Feb 15, 2024
| @@ -24,48 +24,3 @@ def test_show_some(make_df, valid_data, data_source): | |||
| elif variant == "arrow": | |||
| assert df_display.preview.dataframe_num_rows == len(valid_data) | |||
| assert df_display.num_rows == 1 | |||
|
|
|||
|
|
|||
| def test_show_from_cached_collect(make_df, valid_data): | |||
There was a problem hiding this comment.
concept of 'cached_collect' doesn't exist anymore with this PR so i removed these tests
There was a problem hiding this comment.
I guess "cached" would be if we called __repr__() twice in a row now?
jaychia
approved these changes
Feb 20, 2024
| @@ -24,48 +24,3 @@ def test_show_some(make_df, valid_data, data_source): | |||
| elif variant == "arrow": | |||
| assert df_display.preview.dataframe_num_rows == len(valid_data) | |||
| assert df_display.num_rows == 1 | |||
|
|
|||
|
|
|||
| def test_show_from_cached_collect(make_df, valid_data): | |||
There was a problem hiding this comment.
I guess "cached" would be if we called __repr__() twice in a row now?
colin-ho
force-pushed
the
colin/populate_preview_during_repr
branch
from
c6c30a5
to
38fa51b
Compare
samster25
pushed a commit
that referenced
this pull request
Feb 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1858
Closes #1859
Changes:
.collect()_populate_previewmethod thats called duringreprorshow()