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

Unify naming for separator/delimiter in concat operations #13786

Open
coinflip112 opened this issue Jan 17, 2024 · 3 comments · May be fixed by #18428
Open

Unify naming for separator/delimiter in concat operations #13786

coinflip112 opened this issue Jan 17, 2024 · 3 comments · May be fixed by #18428
Labels
enhancement New feature or an improvement of an existing feature

Comments

@coinflip112
Copy link
Contributor

Description

Expr.str.concat has a delimiter argument while pl.concat_str has a separator argument. Conceptually they represent the same thing. Maybe this could be unified 🙏 ?

Willing to do it if somebody provides guidance/reference on argument deprecation wording etc. (will have time over the weekend the earliest).
@coinflip112 coinflip112 added the enhancement New feature or an improvement of an existing feature label Jan 17, 2024
@mcrumiller
Copy link
Contributor

mcrumiller commented Jan 17, 2024 •
edited
Loading

I think separator makes more sense for concatenation. delimiter usually differentiates what's "in" and what's "out", whereas a separator is what goes in between items of a sequence. In this case, the thing-in-between is part of the string, hence separator fits better.

@coinflip112
Copy link
Contributor Author

I guess I agree. However a quick search through the docs shows that separator is used more often to refer to separating generated column names 🤷

@colin-kerkhof
Copy link

Considering the discussion in #7533 it seems to make sense to stick to separator as the argument.

I'll try to make a pull request for that change, hopefully somewhere today.

@colin-kerkhof colin-kerkhof linked a pull request Aug 28, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or an improvement of an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(python,rust): Rename delimiter arg to separator colin-kerkhof/polars
3 participants
@mcrumiller @coinflip112 @colin-kerkhof