posit-dev / great-tables

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

fix: the display issue of integerlike values in nanoplots #319

Closed jrycw closed 2 months ago

jrycw commented 2 months ago

Fix #279 . This PR aims to improve the display of integer or integerlike values in nanoplots. During the implementation, I came across two interesting findings in the code:

demo

from great_tables import GT
import polars as pl

random_numbers_df = pl.DataFrame(
    {
        "i": range(1, 5),
        "lines": [
            "20 23 6 7 37 23 21 4 7 16",
            "2.3 6.8 9.2 2.42 3.5 12.1 5.3 3.6 7.2 3.74",
            "-12 -5 6 3.7 0 8 -7.4",
            "-12 0 15 7 8 10 1 24 17 13 6", # negative values should be ok
        ],
    }
).with_columns(bars=pl.col("lines"))

(
    GT(random_numbers_df, rowname_col="i")
    .fmt_nanoplot(columns="lines")
    .fmt_nanoplot(columns="bars", plot_type="bar")
)

image

image

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 96.55172% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 82.89%. Comparing base (38033ce) to head (dbb528c). Report is 5 commits behind head on main.

Files Patch % Lines
great_tables/_utils_nanoplots.py 96.55% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #319 +/- ## ========================================== + Coverage 82.45% 82.89% +0.43% ========================================== Files 41 41 Lines 4258 4290 +32 ========================================== + Hits 3511 3556 +45 + Misses 747 734 -13 ```

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

machow commented 2 months ago

@rich-iannone do you mind taking a look? The test cases look good, but I'm a bit less familiar with this piece!

rich-iannone commented 2 months ago

This looks great @jrycw ! I also took the opportunity to make the y-axis guides render in the same fashion (stripping decimal part of low and high value if all values of y are integerlike).

Here's what it looks like with my last change applied:

y-axis-as-integer-vals

I used the following table code in my tests to examine the rendered values:

from great_tables import GT
import polars as pl

random_numbers_df = pl.DataFrame(
    {
        "i": range(1, 6),
        "lines": [
            "20 23 6 7 37 23 21 4 7 16",  # integer like
            "2.3 6.8 9.2 2.42 3.5 12.1 5.3 3.6 7.2 3.74",  # two decimal places
            "-12 -5 6 3.7 0 8 -7.4",  # not integerlike, one decimal place
            "-12 0 15 7 8 10 1 24 17 13 6",  # negative values should be ok
            "5.23e-11 15E-12 7E-12 8e-12 10e-12 1e-12 24e-12 17e-12 13e-12 6e-12",  # small values
        ],
    }
).with_columns(bars=pl.col("lines"))

(
    GT(random_numbers_df, rowname_col="i")
    .fmt_nanoplot(columns="lines")
    .fmt_nanoplot(columns="bars", plot_type="bar")
)

Let me know if the extra assignments look okay in terms of good coding style.

jrycw commented 2 months ago

@rich-iannone, that looks fantastic. Honestly, I had wanted to make similar modifications initially. However, I got a bit lost in the _generate_nanoplot function. Nanoplots is definitely a standout feature of Great Tables, and I truly appreciate it. However, as more features are added, the codebase grows quickly, and it took me a while to figure out my place within each if statement. I feel that we might need to refactor this function into smaller pieces for better maintenance."

rich-iannone commented 2 months ago

Thanks again for (so quickly!) looking at my final changes. Agreed that the nanoplots code could use a refactor and some abstractions, and we'll get there!

Will do a final review now :)