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.

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(rust): Add newline to Aggregate..FROM describe_optimization_plan #5253

Merged
merged 6 commits into from
Oct 21, 2022

Conversation

owrior
Copy link
Contributor

@owrior owrior commented Oct 18, 2022

import polars as pl

df = pl.DataFrame({'a':[0,1],'b':[1,2]})

print(
df.lazy().groupby("a").agg(pl.col("b").mean()).describe_optimized_plan()
)

Now returns:

Aggregate
        [col("b").mean()] BY [col("a")] FROM
   DF ["a", "b"]; PROJECT 2/2 COLUMNS; SELECTION: "None"

Closes #5208

@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature rust Related to Rust Polars labels Oct 18, 2022
@ritchie46
Copy link
Member

Thanks! It is almost there. We should only keep the indentation level.

@owrior
Copy link
Contributor Author

owrior commented Oct 18, 2022

Now returns this:

Aggregate
        [col("b").mean()] BY [col("a")] FROM
          DF ["a", "b"]; PROJECT 2/2 COLUMNS; SELECTION: "None"

} => write!(f, "Aggregate\n\t{:?} BY {:?} FROM {:?}", aggs, keys, input),
} => write!(
f,
"Aggregate\n\t{:?} BY {:?} FROM\n\t{:?}",
Copy link
Member

@ritchie46 ritchie46 Oct 18, 2022

Choose a reason for hiding this comment

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

There is an indent argument that should be used. See the other nodes.

The indentation changes when we get deeper in the query.

@owrior
Copy link
Contributor Author

owrior commented Oct 19, 2022

Does this look ok?

Output:

Aggregate
        [col("b").mean()] BY [col("a")] FROM
          DF ["a", "b"]; PROJECT 2/2 COLUMNS; SELECTION: "None"

} => {
writeln!(
f,
"{:indent$}Aggregate\n\t{:?} BY {:?} FROM",
Copy link
Member

Choose a reason for hiding this comment

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

So the first indentation is correct. But then there is a new line followed by a tab, e.g. \n\t that should also be indented by {:indent} instead of the tab.

"{:indent$}Aggregate\n{:indent}{:?} BY {:?} FROM",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh yes sorry I completely missed that one. Think I've addressed this now.

@ritchie46
Copy link
Member

That's it! Thanks.

@ritchie46 ritchie46 merged commit e4f8b86 into pola-rs:master Oct 21, 2022
@owrior owrior deleted the plan-formatting branch October 30, 2022 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or an improvement of an existing feature rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better formatting for optimized plan with aggregate
2 participants