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 example for Config.set_tbl_width_chars #15566

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

mbuhidar
Copy link
Contributor

@mbuhidar mbuhidar commented Apr 9, 2024

A portion of #13161.

Adds an example for Config.set_tbl.width_chars. Format is similar to other examples in file with before/after tables shown side by side, so skipped doctest. Happy to change the format for the example if preferred though.

@github-actions github-actions bot added documentation Improvements or additions to documentation python Related to Python Polars labels Apr 9, 2024
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.12%. Comparing base (b91dedb) to head (b98c9dd).
Report is 91 commits behind head on main.

❗ Current head b98c9dd differs from pull request most recent head ca1cc9e. Consider uploading reports for the commit ca1cc9e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15566      +/-   ##
==========================================
- Coverage   81.15%   81.12%   -0.03%     
==========================================
  Files        1367     1367              
  Lines      175313   174934     -379     
  Branches     2530     2530              
==========================================
- Hits       142282   141923     -359     
+ Misses      32555    32537      -18     
+ Partials      476      474       -2     

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

Copy link
Member

@stinodego stinodego left a comment

Choose a reason for hiding this comment

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

Thanks, but this is not a very 'useful' example. When would I want to truncate ints/floats like this?

It would be nice to get an example that shows real life usage. Now I'm wondering when you'd want to use this feature...

Note that the option is currently bugged and doesn't really work well: #6447

... }
... )
>>> pl.Config.set_tbl_width_chars(16) # doctest: +SKIP
# ...
Copy link
Member

Choose a reason for hiding this comment

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

Could you put the before/after on a different line of code? Then we can actually run the examples and make sure they are correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I will try a better example that helps rather than hinders readability and put it on different lines so it can be checked with doctest.

Copy link
Collaborator

@alexander-beedie alexander-beedie left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! However, as @stinodego is suggesting, could you modify the example so that, instead of the Config option making the table layout less readable, it makes it more readable? Would probably be clearer to use longer string values to demonstrate this 🤔

@mbuhidar
Copy link
Contributor Author

Thanks @alexander-beedie that is a helpful suggestion and gives me an idea for how to improve the example. I will see what I can do.

Copy link

codspeed-hq bot commented Apr 11, 2024

CodSpeed Performance Report

Merging #15566 will not alter performance

Comparing mbuhidar:set_tbl_width_example (5590150) with main (835d198)

Summary

✅ 22 untouched benchmarks

@stinodego
Copy link
Member

stinodego commented Apr 17, 2024

I honestly think #6447 must be addressed before we can add an example. The result of the example is just clearly wrong.

EDIT: Actually, it's almost correct (13 chars instead of 12). Good enough for now, I suppose.

Thanks for the PR! Good to merge.

@stinodego stinodego closed this Apr 17, 2024
@stinodego stinodego reopened this Apr 17, 2024
@stinodego stinodego merged commit bcb3332 into pola-rs:main Apr 17, 2024
12 checks passed
@mbuhidar mbuhidar deleted the set_tbl_width_example branch April 18, 2024 03:07
@alexander-beedie
Copy link
Collaborator

alexander-beedie commented Apr 19, 2024

EDIT: Actually, it's almost correct (13 chars instead of 12). Good enough for now, I suppose.

FYI: 13 is the hard minimum here as it can't compress the 3-letter dtypes any further ;)
"│ str ┆ str │"

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.

3 participants