Gt plt summary customization

[ScottFB101]

Summary

Thank you for contributing to gt-extras! To make this process easier for everyone, please explain the context and purpose of your contribution. Also, list the changes made to the existing code or documentation.

This PR addresses the 4 out of the 5 enhancements proposed in Issue #146. The one enhancement not addressed was: "Display times instead of just dates". I felt that was deserving of another separate PR.

The changes made to the existing code are rather straightforward, and don't need much background besides knowing they address the 4 enhancements previously referenced. This is my opinion of course, and I'm happy to elaborate more on the code here if there's a want for it.

As for the context and purpose of my contribution, I use so many posit-backed packages and tools, that it only feels right to get involved in contributing to the ecosystem.

Related GitHub Issues and PRs

• Ref: Allow users to customize gt_plt_summary() #146

Checklist

• I understand and agree to the Code of Conduct.
• I have followed the Style Guide for Python Code as best as possible for the submitted code.
• I have added pytest unit tests for any new functionality.

[juleswg23]

Thank you Scott! This is looking really awesome, I really think it upgrades the user experience for this function.

I've made some comments (all pretty small) to places I would consider edits. Take a look, and let me know if you think any of the changes are unreasonable, I'd be happy to consider other angles.

Once you feel like the PR is in a good place, the last thing to add are tests. This could probably be one snapshot test with all the optional parameters turned on, and then a few individual tests each checking one of the new parameters. When you run make test, it should report what lines are covered/uncovered... I'd love to be at full coverage for this PR!

[ScottFB101]

Of course! Happy to help, it's been good practice for me.

Alright I made all the changes, but will admit I'm struggling a bit figuring out the snapshot test (not something I've dealt with before), so I can't get 2 of the 500ish lines of summary.py to get coverage using pytest.

[juleswg23]

I can't get 2 of the 500ish lines of summary.py to get coverage using pytest.

This is getting close! The snapshot test is great, it's not necessarily supposed to hit every branch, just to check a standard example's output. If you run make test-update and commit the test_summary.ambr file in the tests/__snapshots/ directory, those snapshot tests should pass with all future as you have them now in all future make test executions.

To get coverage on the un-hit lines (for example, I am seeing the below line isn't covered by the snapshot) you can write tests similar to the rest of the ones in test_summary.py. So for example, to test the below branch:

# Limiting the number of modes displayed to two at maximum
elif len(mode_val) > 2:
    mode_val = "Greater than 2 Modes"
# Converting to string, then listing together
else:
    mode_val = ", ".join(str(i) for i in sorted(mode_val.to_list()))

We can use tests like:

@pytest.mark.parametrize("DataFrame", [pd.DataFrame, pl.DataFrame])
def test_gt_plt_summary_two_modes(DataFrame):
    df = DataFrame({"numeric": [1, 1, 2, 2, 3]})

    result = gt_plt_summary(df, add_mode=True)
    html = result.as_raw_html()

    assert '<td class="gt_row gt_left">1, 2</td>' in html

@pytest.mark.parametrize("DataFrame", [pd.DataFrame, pl.DataFrame])
def test_gt_plt_summary_greater_than_two_modes(DataFrame):
    df = DataFrame({"numeric": [1, 1, 2, 2, 3, 4, 4]})

    result = gt_plt_summary(df, add_mode=True)
    html = result.as_raw_html()

    assert '<td class="gt_row gt_left">Greater than 2 Modes</td>' in html

I know it's not the flashiest to do, but it would be nice to have these kinds of specific tests for each individual new parameter, and then optionally any edge cases you might want to include. (For example, testing behavior when add_mode=True and show_desc_stats=False.)

[juleswg23]

I am noticing a slight bug with this as I run the tests.

https://github.com/posit-dev/gt-extras/pull/160/checks#step:7:671

Specifically, the color it is checking for has changed (line 671 it's counting the blue fills), because in a previous call to gt_plt_summary(), the globals were updated.

It's not the prettiest fix, but one option is to set color_mapping = COLOR_MAPPING, then update color_mapping and pass it all the way down to the SVG functions.

I'm also open to other workarounds.

[ScottFB101]

How would you feel about using a Pytest fixture that resets to the default color_mapping prior to the each separate test?

[juleswg23]

I don't think it solves the root of the problem.

If I run these three in succession, the third one is colored red, rather than orange.

import pandas as pd
from gt_extras import gt_plt_summary

df = pd.DataFrame({"numeric": [1, 1, 2, 2, 3]})
gt_plt_summary(df)

df = pd.DataFrame({"numeric": [1, 1, 2, 2, 3]})
gt_plt_summary(df, new_color_mapping={"numeric":"red"})

df = pd.DataFrame({"numeric": [1, 1, 2, 2, 3]})
gt_plt_summary(df)

[ScottFB101]

Must of commented in the wrong spot originally, so there may be a similar response in this PR thread.

I could make a Pytest fixture that resets the color mapping to the preset from the summary.py script prior to each test.

[juleswg23]

Yes you could, and it's true that it would make the tests pass. But I'm not sure it would solve the underlying problem.

Take the above example - the user just has to change the colors once and it changes the color mapping for all subsequent calls, which feels like unexpected behavior. For that reason, changing the actual behavior of the function seems like a better approach to me.

[ScottFB101]

I can't get 2 of the 500ish lines of summary.py to get coverage using pytest.

# Limiting the number of modes displayed to two at maximum
elif len(mode_val) > 2:
    mode_val = "Greater than 2 Modes"
# Converting to string, then listing together
else:
    mode_val = ", ".join(str(i) for i in sorted(mode_val.to_list()))

We can use tests like:

@pytest.mark.parametrize("DataFrame", [pd.DataFrame, pl.DataFrame])
def test_gt_plt_summary_two_modes(DataFrame):
    df = DataFrame({"numeric": [1, 1, 2, 2, 3]})

    result = gt_plt_summary(df, add_mode=True)
    html = result.as_raw_html()

    assert '<td class="gt_row gt_left">1, 2</td>' in html

@pytest.mark.parametrize("DataFrame", [pd.DataFrame, pl.DataFrame])
def test_gt_plt_summary_greater_than_two_modes(DataFrame):
    df = DataFrame({"numeric": [1, 1, 2, 2, 3, 4, 4]})

    result = gt_plt_summary(df, add_mode=True)
    html = result.as_raw_html()

    assert '<td class="gt_row gt_left">Greater than 2 Modes</td>' in html

Unfortunately, Pandas handles the Mode column as an object. I guess there's no native string column type? Polars can handle string columns, so the alignment in the data cell varies depending on dataframe type. For example, Pandas results in "gt_right", Polars in "gt_left".

For the time being, I strictly checked that the correct values were in the data cell, but didn't check alignment.

[juleswg23]

Unfortunately, Pandas handles the Mode column as an object. I guess there's no native string column type? Polars can handle string columns, so the alignment in the data cell varies depending on dataframe type. For example, Pandas results in "gt_right", Polars in "gt_left".

For the time being, I strictly checked that the correct values were in the data cell, but didn't check alignment.

Yeah, I'm seeing this too. I'd guess there's some workaround within the dataframe, but we could supersede it and set the following:

gt = gt.cols_align(align="right", columns="Mode")

Just note that's not going to work if the mode column isn't present.

I also don't think it's that bad to leave it as is. In that case, the user can edit the table if they see fit.

[ScottFB101]

Unfortunately, Pandas handles the Mode column as an object. I guess there's no native string column type? Polars can handle string columns, so the alignment in the data cell varies depending on dataframe type. For example, Pandas results in "gt_right", Polars in "gt_left".
For the time being, I strictly checked that the correct values were in the data cell, but didn't check alignment.

Yeah, I'm seeing this too. I'd guess there's some workaround within the dataframe, but we could supersede it and set the following:

gt = gt.cols_align(align="right", columns="Mode")

Just note that's not going to work if the mode column isn't present.

I also don't think it's that bad to leave it as is. In that case, the user can edit the table if they see fit.

I added that code to summary.py in e3d54d8