Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(connect): read/write → csv, write → json #3361

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

andrewgazelka
Copy link
Contributor

No description provided.

@andrewgazelka andrewgazelka marked this pull request as ready for review November 20, 2024 10:34
Copy link
Contributor Author

andrewgazelka commented Nov 20, 2024

@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from f679598 to 90ba716 Compare November 20, 2024 18:25
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from f4621df to c999981 Compare November 20, 2024 18:25
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from 90ba716 to e8b1315 Compare November 20, 2024 18:33
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from c999981 to 0d08baf Compare November 20, 2024 18:33
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from e8b1315 to d769d56 Compare November 20, 2024 18:43
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 0d08baf to b8c0e89 Compare November 20, 2024 18:43
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from d769d56 to a3217f3 Compare November 20, 2024 18:49
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from b8c0e89 to 275b4cf Compare November 20, 2024 18:49
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from a3217f3 to 4248e5c Compare November 20, 2024 19:32
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 275b4cf to 909f53e Compare November 20, 2024 19:32
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from 4248e5c to bcccee0 Compare November 20, 2024 22:14
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 909f53e to c014d60 Compare November 20, 2024 22:14
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from bcccee0 to a5c1596 Compare November 20, 2024 23:29
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from c014d60 to ced7e6e Compare November 20, 2024 23:29
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from a5c1596 to 15d7043 Compare November 21, 2024 00:40
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from ced7e6e to de0f611 Compare November 21, 2024 00:40
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from 15d7043 to 10a2505 Compare November 21, 2024 01:45
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from de0f611 to 20f6a2b Compare November 21, 2024 01:45
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from 10a2505 to b8440ed Compare November 21, 2024 01:51
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 20f6a2b to f950b00 Compare November 21, 2024 01:51
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from b8440ed to 7827d10 Compare November 21, 2024 03:21
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from f950b00 to 1036a19 Compare November 21, 2024 03:21
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch 2 times, most recently from 59558e8 to 4dd1993 Compare November 21, 2024 04:20
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch 2 times, most recently from b8cc497 to 48e26c9 Compare November 21, 2024 04:24
@andrewgazelka andrewgazelka force-pushed the andrew/connect-parquet3 branch from 4dd1993 to 4f5ac72 Compare November 21, 2024 05:13
@andrewgazelka andrewgazelka changed the base branch from andrew/connect-parquet3 to graphite-base/3361 December 11, 2024 19:17
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 9dcfb71 to f1ceddb Compare December 11, 2024 21:04
@andrewgazelka andrewgazelka changed the base branch from graphite-base/3361 to main December 11, 2024 21:04
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from f1ceddb to 1a537a7 Compare December 11, 2024 21:11
@andrewgazelka andrewgazelka marked this pull request as ready for review December 11, 2024 21:14
Copy link

codspeed-hq bot commented Dec 11, 2024

CodSpeed Performance Report

Merging #3361 will degrade performances by 34.82%

Comparing andrew/connect-csv-json (59c7803) with main (ea8f8bd)

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 25 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main andrew/connect-csv-json Change
test_iter_rows_first_row[100 Small Files] 345.2 ms 207.1 ms +66.67%
test_show[100 Small Files] 15.7 ms 24.1 ms -34.82%

@andrewgazelka andrewgazelka changed the title [FEAT] connect: read/write -> csv, write -> json feat(connect): read/write -> csv, write -> json Dec 11, 2024
@github-actions github-actions bot added the feat label Dec 11, 2024
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.84%. Comparing base (ea8f8bd) to head (59c7803).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...t/src/translation/logical_plan/read/data_source.rs 71.42% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3361      +/-   ##
==========================================
+ Coverage   77.80%   77.84%   +0.03%     
==========================================
  Files         718      718              
  Lines       88247    88250       +3     
==========================================
+ Hits        68662    68698      +36     
+ Misses      19585    19552      -33     
Files with missing lines Coverage Δ
src/daft-connect/src/op/execute/write.rs 80.00% <100.00%> (+1.95%) ⬆️
...t/src/translation/logical_plan/read/data_source.rs 81.08% <71.42%> (-3.30%) ⬇️

... and 4 files with indirect coverage changes

@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 1a537a7 to d8cc857 Compare December 11, 2024 22:02
@andrewgazelka andrewgazelka requested review from universalmind303 and removed request for colin-ho December 11, 2024 23:44
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch 5 times, most recently from 597ef2d to b9fc577 Compare December 19, 2024 02:44
@andrewgazelka andrewgazelka changed the title feat(connect): read/write -> csv, write -> json feat(connect): read/write → csv, write → json Dec 19, 2024
@andrewgazelka andrewgazelka force-pushed the andrew/connect-csv-json branch from 87c8e55 to 59c7803 Compare December 19, 2024 10:14
Copy link
Contributor Author

@andrewgazelka andrewgazelka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

src/daft-connect/src/op/execute/write.rs Outdated Show resolved Hide resolved
tests/connect/test_csv.py Outdated Show resolved Hide resolved
Copy link
Contributor

@raunakab raunakab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Tests seem straightforward too.

@andrewgazelka andrewgazelka enabled auto-merge (squash) December 19, 2024 10:24
@andrewgazelka andrewgazelka merged commit ae74c10 into main Dec 19, 2024
42 of 44 checks passed
@andrewgazelka andrewgazelka deleted the andrew/connect-csv-json branch December 19, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants