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

[FEA] Remove column_names from table_metadata #6411

Closed
nvdbaranec opened this issue Oct 2, 2020 · 6 comments · Fixed by #12578
Closed

[FEA] Remove column_names from table_metadata #6411

nvdbaranec opened this issue Oct 2, 2020 · 6 comments · Fixed by #12578
Labels
cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API.

Comments

@nvdbaranec
Copy link
Contributor

nvdbaranec commented Oct 2, 2020 •
edited
Loading

This PR (#6318) adds a new field to the table_metadata struct, schema_info, which contains the column names for the entire hierarchy of returned columns, not just the root columns.

This is semi-redundant with the existing column_names but we decided to avoid breaking any existing external dependencies that use column_names for 0.16.

We should decide if they should be folded together (likely, column_names would just use the new data structure)
@nvdbaranec nvdbaranec added feature request New feature or request Needs Triage Need team to review and classify 4 - Needs cuIO Reviewer labels Oct 2, 2020
@kkraus14 kkraus14 added Python Affects Python cuDF API. cuIO cuIO issue libcudf Affects libcudf (C++/CUDA) code. and removed 4 - Needs cuIO Reviewer Needs Triage Need team to review and classify labels Oct 8, 2020
@vuule
Copy link
Contributor

vuule commented Jan 29, 2021

@devavret this seems related to schema changes

@devavret
Copy link
Contributor

Thanks for pointing to the issue. I observed this but didn't know its already filed.

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@vuule
Copy link
Contributor

vuule commented Mar 16, 2021

still relevant

@vuule
Copy link
Contributor

vuule commented Sep 7, 2022

From some work in the JSON reader, it looks like we can remove column_names, as the same names can be accessed in schema_info.
It should just be a matter of switching to the new member in Cython and in the C++ tests.
@galipremsagar any concerns with gradual switch to schema_info?

@galipremsagar
Copy link
Contributor

No concerns from my side, SGTM 👍

@vuule vuule changed the title [FEA] Resolve what to do with semi-redundant fields in cuIO table_metadata [FEA] Remove column_names from table_metadata Sep 27, 2022
This was referenced Oct 21, 2022
Merged
Closed
rapids-bot bot pushed a commit that referenced this issue Oct 31, 2022
@vuule
Pass column names to write_csv instead of table_metadata pointer (#…
f0b4c4f 
…11972)

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`.

Authors:
  - Vukasin Milovanovic (https://github.com/vuule)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Matthew Roeschke (https://github.com/mroeschke)
  - Nghia Truong (https://github.com/ttnghia)
  - Yunsong Wang (https://github.com/PointKernel)

URL: #11972
@vuule vuule mentioned this issue Jan 19, 2023
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this issue Jan 24, 2023
@vuule
Remove column names (#12578)
9c9862f 
Closes #6411

Removed `column_names` in favor of `schema_info`.

Authors:
  - Vukasin Milovanovic (https://github.com/vuule)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Elias Stehle (https://github.com/elstehle)
  - Karthikeyan (https://github.com/karthikeyann)
  - Bradley Dice (https://github.com/bdice)
  - Matthew Roeschke (https://github.com/mroeschke)

URL: #12578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove column names vuule/cudf
6 participants
@devavret @kkraus14 @galipremsagar @GregoryKimball @vuule @nvdbaranec