Skip to content

Add rename columns and get column names #106

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

Merged

Conversation

MarcoGorelli
Copy link
Contributor

@MarcoGorelli MarcoGorelli commented Mar 14, 2023

closes #105

Doesn't look like there's objections, so I went ahead and opened this

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks @MarcoGorelli. One question: do we expect/allow users to mutate the returned list? If not, a tuple of strings may be nicer (can return an existing tuple perhaps, a list must be copied) and may be slightly nicer for a JIT compiler.

@MarcoGorelli
Copy link
Contributor Author

yup, sounds good, have updated

@rgommers
Copy link
Member

A bit more discussion on the return type:

  • For really wide dataframes, @kkraus14 pointed out that a Python objects like a tuple may be limiting. We decided to punt on this though, since there's no clear alternative. Implementers could return some other object that has tuple-compatible semantics.
  • Some discussion about allowing returning Sequence[str, ...] instead. Assuming Mypy is happy with that, that seems like an improvement.

MarcoGorelli and others added 2 commits March 17, 2023 09:32
Co-authored-by: Keith Kraus <keith.j.kraus@gmail.com>
Co-authored-by: Keith Kraus <keith.j.kraus@gmail.com>
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM too. Let's give this a go - thanks @MarcoGorelli and @kkraus14!

@rgommers rgommers merged commit 399b5c4 into data-apis:main Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataFrame.rename_columns, DataFrame.column_names
3 participants