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

docs(python): Add description and example of what happens when ignore_nulls=True on pl.concat_str #16889

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bertiewooster
Copy link
Contributor

Addresses #16814 on https://docs.pola.rs/py-polars/html/reference/expressions/api/polars.concat_str.html#polars.concat_str. (Other than changing the font, which is a larger decision.)

@bertiewooster bertiewooster changed the title Add description and example of what happens when ignore_nulls=True Add description and example of what happens when ignore_nulls=True on pl.concat_str Jun 12, 2024
Copy link

codecov bot commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.84%. Comparing base (0b0af39) to head (3865dcb).
Report is 184 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16889      +/-   ##
==========================================
- Coverage   81.33%   80.84%   -0.49%     
==========================================
  Files        1425     1466      +41     
  Lines      187867   192324    +4457     
  Branches     2702     2745      +43     
==========================================
+ Hits       152797   155481    +2684     
- Misses      34574    36340    +1766     
- Partials      496      503       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bertiewooster bertiewooster marked this pull request as ready for review June 15, 2024 20:30
@bertiewooster bertiewooster changed the title Add description and example of what happens when ignore_nulls=True on pl.concat_str docs(python): Add description and example of what happens when ignore_nulls=True on pl.concat_str Jun 18, 2024
@github-actions github-actions bot added documentation Improvements or additions to documentation python Related to Python Polars labels Jun 18, 2024
@bertiewooster
Copy link
Contributor Author

@DeflateAwning does this address the issue?

@DeflateAwning
Copy link
Contributor

The example looks great! Assuming executing that example works as indicated, I believe your description is actually wrong.

You wrote:

If True, null values will be propagated and will appear to be empty strings.

In reality:

If True, null values are removed from the input list before concatenation.

Issues:

  • "Propogated" means "passed up to the caller, thus making the output cell null"
  • "Appear to be empty strings" would mean that the delimeter is duplicated.

@bertiewooster bertiewooster marked this pull request as draft June 27, 2024 21:36
@bertiewooster
Copy link
Contributor Author

bertiewooster commented Jun 27, 2024

Good catch; I wasn't thinking about the separator. I corrected the description.
BTW, to pass the doctests, the code example output has to be correct.

@bertiewooster bertiewooster marked this pull request as ready for review June 27, 2024 22:32
Copy link
Contributor

@DeflateAwning DeflateAwning left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@bertiewooster
Copy link
Contributor Author

Request final review and, if approved, merging on this PR. It was endorsed by the original requestor. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants