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

fix(python, rust): properly interpret FMT_MAX_ROWS - remove arbitrary minimum, fix Series formatting #5281

Merged
merged 2 commits into from
Oct 21, 2022
Merged

Conversation

alexander-beedie
Copy link
Collaborator

@alexander-beedie alexander-beedie commented Oct 21, 2022

Some slightly OCD updates/fixes:

  • Fixed a few issues with use of FMT_MAX_ROWS (Rust)

    • Removed arbitrary minimum value:

      Min value was forcibly capped at 2, due to the formatting code requiring at least one row before and one row after any ... placeholders (which represent extant but non-displayed rows/cells); adjusted things so it properly handles 0 and 1.

      Not many compelling reasons to want to set such small values, but equally there are no good reasons not to allow it if somebody decides they do want to :)

      # -----------------+------------------------------------
      # BEFORE           | AFTER 
      # (1/0 fmt like 2) | (1/0 format as expected)
      # -----------------+------------------------------------
      #                  |
      # shape: (3, 2)    |    shape: (3, 2)    shape: (3, 2)  
      # ┌─────┬─────┐    |    ┌─────┬─────┐    ┌─────┬─────┐
      # │ s   ┆ n   │    |    │ s   ┆ n   │    │ s   ┆ n   │
      # │ --- ┆ --- │    |    │ --- ┆ --- │    │ --- ┆ --- │
      # │ str ┆ i64 │    |    │ str ┆ i64 │    │ str ┆ i64 │
      # ╞═════╪═════╡    |    ╞═════╪═════╡    ╞═════╪═════╡
      # │ a   ┆ 1   │    |    │ a   ┆ 1   │    │ ... ┆ ... │
      # ├╌╌╌╌╌┼╌╌╌╌╌┤    |    ├╌╌╌╌╌┼╌╌╌╌╌┤    └─────┴─────┘
      # │ ... ┆ ... │    |    │ ... ┆ ... │
      # ├╌╌╌╌╌┼╌╌╌╌╌┤    |    └─────┴─────┘
      # │ c   ┆ 3   │    |
      # └─────┴─────┘    |
    • Fixed Series formatting for certain values:

      Certain values of FMT_MAX_ROWS resulted in incorrect formatting (omitting a value), due to integer division shenanigans; wasn't previously noticed as the default is a small even number. Fixed the array formatting macro that calculates what to display (in addition to also handling 0 and 1 values, as above).

      Example: when FMT_MAX_ROWS = 3 →

      # ---------------------------------------+----------------------
      # BEFORE                                 |  AFTER 
      # (df is correct)  (series missing "3")  |  (series matches df)
      # ---------------------------------------+----------------------
      #                                        |
      # shape: (4, 1)     shape: (4,)          |   shape: (4,)
      # ┌─────┐           Series: 'n' [i64]    |   Series: 'n' [i64]
      # │ n   │           [                    |   [ 
      # │ --- │               1                |       1 
      # │ i64 │               ...              |       ...
      # ╞═════╡               4                |       3
      # │ 1   │           ]                    |       4
      # │ ... │                                |   ]
      # │ 3   │                                |
      # │ 4   │                                |
      # └─────┘                                |
  • Consolidated config vars (Rust)

    Noticed that there is an existing config.rs, but it was never updated with the newer table formatting variables; updated accordingly so we don't have a mix of bare strings and const refs.

  • Exposed new UTF_FULL_CONDENSED comfy-table preset (Rust, Python)
    (which I added there earlier in the week to help reclaim some vertical space ;)

    >>> pl.Config.set_tbl_formatting("UTF8_FULL_CONDENSED")
    
    # UTF_FULL:            UTF_FULL_CONDENSED:
    # ┌───────┬───────┐    ┌───────┬───────┐
    # │ Hello ┆ there │    │ Hello ┆ there │
    # ╞═══════╪═══════╡    ╞═══════╪═══════╡
    # │ a     ┆ b     │    │ a     ┆ b     │
    # ├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤    │ c     ┆ d     │
    # │ c     ┆ d     │    │ e     ┆ f     │
    # ├╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┤    └───────┴───────┘
    # │ e     ┆ f     │ 
    # └───────┴───────┘    
  • Misc: fixed Expr docstring examples for round and truncate (Python)
    The examples in these two expressions were directly copied from Series instead of actually showing the relevant expressions in use; updated appropriately.

@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Oct 21, 2022
@alexander-beedie alexander-beedie changed the title fix(python, rust): improve interpretation of FMT_MAX_ROWS - remove arbitrary minimum value, fix Series formatting for odd-numbered values fix(python, rust): properly interpret FMT_MAX_ROWS - remove arbitrary minimum, fix Series formatting for odd-numbered values Oct 21, 2022
@alexander-beedie alexander-beedie changed the title fix(python, rust): properly interpret FMT_MAX_ROWS - remove arbitrary minimum, fix Series formatting for odd-numbered values fix(python, rust): properly interpret FMT_MAX_ROWS - remove arbitrary minimum, fix Series formatting Oct 21, 2022
@alexander-beedie
Copy link
Collaborator Author

alexander-beedie commented Oct 21, 2022

Looks like a few doctests need updating as a result of fixing Series formatting...

Updated: done.

@ghuls
Copy link
Collaborator

ghuls commented Oct 21, 2022

@alexander-beedie comfy-table version is also updated a few days ago in Polars, so it is possible that there is more useful stuff in the 6.x release: https://github.com/Nukesor/comfy-table/blob/main/CHANGELOG.md

@alexander-beedie
Copy link
Collaborator Author

alexander-beedie commented Oct 21, 2022

@alexander-beedie comfy-table version is also updated a few days ago in Polars, so it is possible that there is more useful stuff in the 6.x release: https://github.com/Nukesor/comfy-table/blob/main/CHANGELOG.md

Yup, I'm curious if there's anything else we can do with it...
(I saw you'd already taken advantage of the new add_rows method :)

@ritchie46 ritchie46 merged commit 1a8ad9a into pola-rs:master Oct 21, 2022
@alexander-beedie alexander-beedie deleted the table-formatting branch October 25, 2022 20:03
zundertj pushed a commit to zundertj/polars that referenced this pull request Jan 7, 2023
@alexander-beedie alexander-beedie mentioned this pull request Mar 1, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants