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.

Sign up for GitHub

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

Jump to bottom

Pass column names to write_csv instead of table_metadata pointer #11972

Merged
merged 8 commits into from
Oct 31, 2022
Merged

Pass column names to write_csv instead of table_metadata pointer #11972

merged 8 commits into from
Oct 31, 2022

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Oct 21, 2022 •
edited
Loading

Description

contributes to #6411
write_csv takes a pointer to table_metadata but only uses the column names.
This PR changes the API to directly take column names. This also aligns with read_csv.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vuule vuule added cuIO cuIO issue improvement Improvement / enhancement to an existing function breaking Breaking change labels Oct 21, 2022
@vuule vuule self-assigned this Oct 21, 2022
@github-actions github-actions bot added Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Oct 21, 2022
@codecov
Copy link

codecov bot commented Oct 22, 2022 •
edited
Loading

Codecov Report

Base: 87.40% // Head: 88.12% // Increases project coverage by +0.71% 🎉

Coverage data is based on head (e99e931) compared to base (f72c4ce).
Patch has no changes to coverable lines.

❗ Current head e99e931 differs from pull request most recent head 8cf2b5a. Consider uploading reports for the commit 8cf2b5a to get more accurate results

Additional details and impacted files 
@@               Coverage Diff                @@
##           branch-22.12   #11972      +/-   ##
================================================
+ Coverage         87.40%   88.12%   +0.71%     
================================================
  Files               133      133              
  Lines             21833    21982     +149     
================================================
+ Hits              19084    19372     +288     
+ Misses             2749     2610     -139
Impacted Files Coverage Δ
python/strings_udf/strings_udf/__init__.py 84.31% <0.00%> (-12.57%) ⬇️
python/cudf/cudf/io/text.py 91.66% <0.00%> (-8.34%) ⬇️
python/cudf/cudf/core/_base_index.py 81.28% <0.00%> (-4.27%) ⬇️
python/strings_udf/strings_udf/_typing.py 94.73% <0.00%> (-1.06%) ⬇️
python/cudf/cudf/utils/utils.py 89.91% <0.00%> (-0.69%) ⬇️
python/cudf/cudf/testing/dataset_generator.py 72.83% <0.00%> (-0.42%) ⬇️
python/dask_cudf/dask_cudf/core.py 73.72% <0.00%> (-0.41%) ⬇️
python/dask_cudf/dask_cudf/backends.py 84.90% <0.00%> (-0.37%) ⬇️
python/cudf/cudf/core/dataframe.py 93.67% <0.00%> (-0.10%) ⬇️
python/cudf/cudf/io/orc.py 92.94% <0.00%> (-0.09%) ⬇️
... and 24 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@vuule vuule changed the title write_csv takes column names instead of table_metadata pointer Pass column names to write_csv instead of table_metadata pointer Oct 24, 2022
@vuule vuule marked this pull request as ready for review October 24, 2022 19:06
@vuule vuule requested review from a team as code owners October 24, 2022 19:06
@vuule vuule requested a review from ttnghia October 24, 2022 22:58
@vuule
Copy link
Contributor Author

vuule commented Oct 29, 2022

rerun tests

PointKernel
PointKernel approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@vuule vuule added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Oct 31, 2022
@vuule
Copy link
Contributor Author

vuule commented Oct 31, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f0b4c4f into rapidsai:branch-22.12 Oct 31, 2022
@vuule vuule deleted the write_csv-col-names branch October 31, 2022 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttnghia ttnghia ttnghia approved these changes

@mroeschke mroeschke mroeschke approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@PointKernel PointKernel PointKernel approved these changes

@skirui-source skirui-source Awaiting requested review from skirui-source skirui-source is a code owner automatically assigned from rapidsai/cudf-python-codeowners

@cwharris cwharris Awaiting requested review from cwharris cwharris is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@vuule vuule

Labels
5 - Ready to Merge Testing and reviews complete, ready to merge breaking Breaking change cuIO cuIO issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API.
Projects
cuDF/Dask/Numba/UCX
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vuule @ttnghia @mroeschke @galipremsagar @PointKernel