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

Remove column names #12578

Merged
merged 11 commits into from
Jan 24, 2023
Merged

Remove column names #12578

merged 11 commits into from
Jan 24, 2023

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Jan 19, 2023

Description

Closes #6411

Removed column_names in favor of schema_info.

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 tech debt breaking Breaking change labels Jan 19, 2023
@vuule vuule self-assigned this Jan 19, 2023
@github-actions github-actions bot added Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Jan 19, 2023
@vuule vuule added the improvement Improvement / enhancement to an existing function label Jan 19, 2023
karthikeyann added a commit to karthikeyann/cudf that referenced this pull request Jan 19, 2023
@karthikeyann
remove column_names usage (ref rapidsai#12578)
1c6821d
@codecov
Copy link

codecov bot commented Jan 20, 2023 •
edited
Loading

Codecov Report

Base: 86.58% // Head: 85.71% // Decreases project coverage by -0.87% ⚠️

Coverage data is based on head (47a9679) compared to base (b6dccb3).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@               Coverage Diff                @@
##           branch-23.02   #12578      +/-   ##
================================================
- Coverage         86.58%   85.71%   -0.87%     
================================================
  Files               155      155              
  Lines             24368    24865     +497     
================================================
+ Hits              21098    21312     +214     
- Misses             3270     3553     +283
Impacted Files Coverage Δ
python/cudf/cudf/_version.py 1.41% <0.00%> (-98.59%) ⬇️
python/cudf/cudf/core/buffer/spill_manager.py 72.50% <0.00%> (-7.50%) ⬇️
python/cudf/cudf/core/buffer/spillable_buffer.py 91.07% <0.00%> (-1.78%) ⬇️
python/cudf/cudf/utils/dtypes.py 77.85% <0.00%> (-1.61%) ⬇️
python/cudf/cudf/options.py 86.11% <0.00%> (-1.59%) ⬇️
python/cudf/cudf/core/single_column_frame.py 94.30% <0.00%> (-1.27%) ⬇️
...ython/custreamz/custreamz/tests/test_dataframes.py 98.38% <0.00%> (-1.01%) ⬇️
python/dask_cudf/dask_cudf/io/csv.py 96.34% <0.00%> (-1.00%) ⬇️
python/dask_cudf/dask_cudf/io/parquet.py 91.81% <0.00%> (-0.59%) ⬇️
python/cudf/cudf/core/multiindex.py 91.66% <0.00%> (-0.51%) ⬇️
... and 45 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 marked this pull request as ready for review January 23, 2023 09:02
@vuule vuule requested review from a team as code owners January 23, 2023 09:02
elstehle
elstehle approved these changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@elstehle elstehle left a comment

Choose a reason for hiding this comment

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

C++ changes LGTM

karthikeyann
karthikeyann approved these changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@karthikeyann karthikeyann 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.

cpp/include/cudf/io/types.hpp
@@ -144,7 +144,6 @@ struct column_name_info {
* f5 f6
*/
struct table_metadata {
std::vector<std::string> column_names; //!< Names of columns contained in the table
Copy link
Contributor

Choose a reason for hiding this comment

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

Doc has usage of column_names. That may need update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! removed the outdated docs

@vuule vuule added the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jan 23, 2023
@vuule vuule requested a review from a team as a code owner January 23, 2023 19:34
@github-actions github-actions bot added the Java Affects Java cuDF API. label Jan 23, 2023
@vuule vuule changed the base branch from branch-23.02 to branch-23.04 January 23, 2023 23:37
@vuule vuule changed the base branch from branch-23.04 to branch-23.02 January 23, 2023 23:37
@vuule vuule removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jan 23, 2023
@vuule
Copy link
Contributor Author

vuule commented Jan 24, 2023

/merge

@rapids-bot rapids-bot bot merged commit 9c9862f into rapidsai:branch-23.02 Jan 24, 2023
@vuule vuule deleted the remove-column_names branch January 24, 2023 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galipremsagar galipremsagar galipremsagar approved these changes

@mroeschke mroeschke mroeschke approved these changes

@karthikeyann karthikeyann karthikeyann approved these changes

@elstehle elstehle elstehle approved these changes

@bdice bdice bdice approved these changes

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

Assignees

@vuule vuule

Labels
breaking Breaking change cuIO cuIO issue improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Remove column_names from table_metadata
6 participants
@vuule @bdice @elstehle @karthikeyann @mroeschke @galipremsagar