posit-dev / great-tables

Make awesome display tables using Python.
https://posit-dev.github.io/great-tables/
MIT License
1.42k stars 48 forks source link

Feat init methods #371

Closed machow closed 3 weeks ago

machow commented 1 month ago

This WIP PR will enable options that currently must be set on construction of GT() to be set with methods.

The options are...

The most challenging options are rowname_col and groupname_col, since setting these options means checking, and potentially undoing a number of things.

For example, a column that is being set to rowname_col, should:

Similarly, a new groupname_col will need to remove any styles that were set on it (when it was in the body).

Main changes

This PR required a dive into components like Stub. While I was there, it seemed like a good idea to consolidate things into it, so that GT can't get into funky states, and it's clear which class drives things like final row order. This makes things a bit less noodly when updating things like groupname_cols(), since the Stub and Boxhead can now handle the whole task!

Big items:

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 95.77465% with 6 lines in your changes missing coverage. Please review.

Project coverage is 86.77%. Comparing base (a2ecbbe) to head (0426c83). Report is 61 commits behind head on main.

Files Patch % Lines
great_tables/_gt_data.py 96.70% 3 Missing :warning:
great_tables/utils_render_common.py 66.66% 2 Missing :warning:
great_tables/_modify_rows.py 96.87% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #371 +/- ## ========================================== + Coverage 86.20% 86.77% +0.57% ========================================== Files 41 42 +1 Lines 4348 4545 +197 ========================================== + Hits 3748 3944 +196 - Misses 600 601 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

machow commented 3 weeks ago

@rich-iannone, WDYT of these alternative approaches for method names:

  1. the current approach, all init args now available via .with_*() methods)
    • .with_rowname_col(name)
    • .with_groupname_col(name)
    • .with_locale(locale)
    • .with_id(id)
    • .with_autoalign(...)
  2. exposing something like .tab_stub(rowname_col=..., groupname_col=...)
    • .tab_stub(...)
    • .with_locale(...)
    • .with_id(...)
    • .with_autoalign(...)
  3. alternatively, could name other things with_context(...) or something...
    • .tab_stub(...)
    • .with_context(locale=, id=, autoalign=)

My suspicion is that (1) will start to feel cumbersome, because setting both row and group names feels very common. I like that (2) hoists that activity up to a top-level structure activity (.tab_stub()), with the other tab methods. However, the consistency of just using with_ seems nice? 🤷🏼

Two key considerations

Consistency

It seems like .tab_stub() is consistent with the structure method names. A benefit of this is people will encounter it very early in the reference, which seems reflective of how commonly it's used:

image

There will also need to be a loc.stub(), which will reinforce .tab_stub().

Newline-ness

Using common formatters, each .with_() call will be on its own line. This doesn't seem too bad for less common options (like .with_locale() or .with_id()), but seems funky with rowname_col and groupname_col...

from great_tables import GT, exibble
(
    GT(exibble)
    .with_rowname_col("row")
    .with_groupname_col("group")
    .tab_header(title = "Some title", subtitle = "Some subtitle")

    # formatters ----
    .fmt(...)
)
machow commented 3 weeks ago

One smaller piece to resolve: auto_align is currently set to True. So it's possible to expose options to not auto_align in the final render, but that won't stop GT(some_data) from running alignment logic. We might need some kind of global option (or to simplify the auto_align logic; or just let it ride for now since auto_align is very helpful for display).

rich-iannone commented 3 weeks ago

I really like option 2. Feels like the right balance between convenience and naming consistency!

machow commented 3 weeks ago

Alright I think the key stages of this PR were:

Happy to go through and comment things out, since I know it's a fairly sweeping set of changes 😓

machow commented 3 weeks ago

I tucked in some quick tweaks to docs to mention .tab_stub(), and swapped it into our examples!