-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat: Increase verbosity of duplicate column error message #11899
Conversation
I cannot replicate the hypothesis error. It appears it was failing in this case:
This should pass the assertion test, no clue why it's reporting 2. I installed the same hypothesis version and set the replication decorator, but it fails to run on my machine. |
It's this: #11910 |
Thanks Alex. Once that gets resolved I'll pull those changes in here. |
@ritchie46 this one is now good to go. |
crates/polars-core/src/frame/mod.rs
Outdated
@@ -1518,7 +1518,14 @@ impl DataFrame { | |||
let mut names = PlHashSet::with_capacity(cols.len()); | |||
for name in cols { | |||
if !names.insert(name.as_str()) { | |||
polars_bail!(duplicate = name); | |||
let msg = format!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still to broad IMO. I think this should be here:
polars_ensure!(names.insert(name), duplicate = name); |
@mcrumiller I opened a new issue specifically for the improvement in this PR. Could you address Ritchie's comment and finish this improvement? |
@stinodego I think I accomplished what he suggested, let me know if it still needs work and/or you have any suggestions. Edit: yikes hang on, there's some bad formatting. |
979645e
to
a836380
Compare
Edit: fixed. Got some failures due to the new CI issue. |
Should we add newlines for our long error messages, or let the terminal wrap them? >>> import polars as pl
>>> df = pl.DataFrame({"a": [1], "b": [1]})
>>> df.with_columns(pl.all().alias("a"))
polars.exceptions.ComputeError: the name: 'a' passed to `LazyFrame.with_columns` is duplicate
It's possible that multiple expressions are returning the same default column name. If this is the case, try renaming the columns with `.alias("new_name")` to avoid duplicate column names.
>>> df.lazy().with_columns(pl.all().alias("a")).collect()
polars.exceptions.ComputeError: the name: 'a' passed to `LazyFrame.with_columns` is duplicate
It's possible that multiple expressions are returning the same default column name. If this is the case, try renaming the columns with `.alias("new_name")` to avoid duplicate column names. |
Terminal should wrap them. |
crates/polars-lazy/src/physical_plan/executors/projection_utils.rs
Outdated
Show resolved
Hide resolved
I'll rebase to fix the CI tests. |
c478a1b
to
3690029
Compare
Resolves #14458
I notice that none of our rust errors are multi-line, is this okay here?