Enhance test coverage for column selection #325
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #325 +/- ##
==========================================
+ Coverage 82.87% 83.43% +0.55%
==========================================
Files 41 41
Lines 4287 4287
==========================================
+ Hits 3553 3577 +24
+ Misses 734 710 -24 ☔ View full report in Codecov by Sentry. |
| @@ -231,3 +197,75 @@ def test_cols_width_fully_set_pct_2(): | |||
| assert gt_tbl._boxhead[0].column_width == "10%" | |||
| assert gt_tbl._boxhead[1].column_width == "10%" | |||
| assert gt_tbl._boxhead[2].column_width == "40%" | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("df_lib, columns", [(pd, "a"), (pl, cs.starts_with("a"))]) | |||
There was a problem hiding this comment.
WDYT of making df_lib a fixture, that passes in the dataframe directly? Similar to this fixture in tbl_data tests
https://github.com/posit-dev/great-tables/blob/main/tests/test_tbl_data.py#L19-L25
There was a problem hiding this comment.
Please correct me if I'm mistaken, but it appears that the current test case will generate two small test cases for each test function. However, if we do:
params_frames = [pytest.param(pd.DataFrame, id="pandas"), pytest.param(pl.DataFrame, id="polars")]
@pytest.fixture(params=params_frames, scope="function")
def df(request):
return request.param({"a": [1, 2], "b": [3, 4], "c": [5, 6], "d": [7, 8]})
@pytest.mark.parametrize("columns", ["a", cs.starts_with("a")])
def test_cols_move_single_col(df, columns):
src_gt = GT(df)
new_gt = cols_move(src_gt, columns=columns, after="b")
assert [col.var for col in new_gt._boxhead] == ["b", "a", "c", "d"]It seems that we'll have four small test cases for each function, resulting in two types of dataframes with two types of columns. It's evident that when Pandas interacts with Polars columns, the test will fail.
tests/test_spanners.py::test_cols_move_single_col[pandas-a] PASSED [ 25%]
tests/test_spanners.py::test_cols_move_single_col[pandas-columns1] FAILED [ 50%]
tests/test_spanners.py::test_cols_move_single_col[polars-a] PASSED [ 75%]
tests/test_spanners.py::test_cols_move_single_col[polars-columns1] PASSED [100%]
=========================================================== short test summary info ============================================================
FAILED tests/test_spanners.py::test_cols_move_single_col[pandas-columns1] - NotImplementedError: Unsupported selection expr: col("^(a).*$")
1 failed, 3 passed, 1 warning in 0.96sThere was a problem hiding this comment.
Ah, I didn't realize this is testing specific combinations, where the suggestion just crosses everything. I seems reasonable to keep it as is!
It seems okay to lean on the tests of https://github.com/posit-dev/great-tables/blob/main/tests/test_formats.py#L74 |
| @@ -231,3 +197,75 @@ def test_cols_width_fully_set_pct_2(): | |||
| assert gt_tbl._boxhead[0].column_width == "10%" | |||
| assert gt_tbl._boxhead[1].column_width == "10%" | |||
| assert gt_tbl._boxhead[2].column_width == "40%" | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("df_lib, columns", [(pd, "a"), (pl, cs.starts_with("a"))]) | |||
There was a problem hiding this comment.
Ah, I didn't realize this is testing specific combinations, where the suggestion just crosses everything. I seems reasonable to keep it as is!
|
Thanks so much for making sure things are covered! @rich-iannone do you want to merge? |
Related to #35.
This PR aims to enhance test coverage for
cols_move,cols_move_to_start,cols_move_to_end, andcols_hidefunctions in bothPandasandPolars.Meanwhile, given the substantial reliance on column selection within the
fmt_*functions, should we conduct thorough tests for them usingPolarsselectors, or are we satisfied with the current state?