From 49f10f008c35ff4cad98ca210886a0be028e88c5 Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Sun, 23 Aug 2026 02:35:21 +0300 Subject: [PATCH 1/3] fix(adorn): treat the first column as a row identifier adorn_pct_formatting, adorn_ns and adorn_rounding selected every numeric column, so a numeric row identifier was formatted as a percentage, had an N count appended, or was rounded. adorn_totals and adorn_percentages already exclude the first column. Apply the same exclusion at the three remaining sites and add tests for a numeric first column, including a tabyl pipeline on an integer grouping variable. --- CHANGELOG.md | 1 + janitor/functions/adorn.py | 43 +++++++++++++++---- tests/functions/test_adorn.py | 81 +++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9877dc507..780550a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - [PERF] Reduce peak memory in `_build_indexer_reorder_contents` for wide frames (reps >= 8) using single NumPy allocation; tall frames with few repetitions retain the original reshape path. - Issue #1655 @Anupam2400 - [ENH] `conditional_join` now picks the most selective `<`/`<=`/`>`/`>=` predicate as its binary-search anchor (instead of the first one supplied) when `keep` is `'first'` or `'last'`, fixing pathological slowdowns from unfavorable predicate ordering; the choice is estimated from a fixed-size sample so the selection cost no longer scales with input size; `keep='all'` output is unaffected. - Issue #1641 @samukweku - [BUG] Fix `conditional_join` crash for `keep='last'` joins with a single `<`/`<=` window and multiple non-equi conditions - a missing `counts` argument to the Rust `index_starts_only_keep_last` call. - Issue #1641 @samukweku +- [BUG] `adorn_pct_formatting`, `adorn_ns` and `adorn_rounding` no longer treat a numeric first column as data. The first column is a row identifier, matching `adorn_totals` and `adorn_percentages`. @dylanpulver - [ENH] Avoid copying column data during `conditional_join` input validation. - Issue #1645, PR #1642 @samukweku - [ENH] Limit `conditional_join` matching work to columns referenced by join diff --git a/janitor/functions/adorn.py b/janitor/functions/adorn.py index 883f26695..ff3d6aff1 100644 --- a/janitor/functions/adorn.py +++ b/janitor/functions/adorn.py @@ -211,6 +211,8 @@ def adorn_pct_formatting( This function formats numeric columns (assumed to be proportions between 0 and 1) as percentage strings with the specified number of decimal places. + The first column is treated as a row identifier and is left untouched, + consistent with `adorn_totals` and `adorn_percentages`. Examples: Format percentages with default settings. @@ -263,8 +265,14 @@ def adorn_pct_formatting( # Preserve original counts if they exist original_counts = df.attrs.get("_original_counts") - # Identify numeric columns - numeric_cols = df.select_dtypes(include=[np.number]).columns.tolist() + # Identify numeric columns, excluding the first column which is treated + # as a row identifier (consistent with R janitor's tabyl behavior) + first_col = df.columns[0] + numeric_cols = [ + col + for col in df.select_dtypes(include=[np.number]).columns.tolist() + if col != first_col + ] rounding_mode = ROUND_HALF_EVEN if rounding == "half to even" else ROUND_HALF_UP quantize_str = f"0.{'0' * digits}" if digits > 0 else "0" @@ -305,7 +313,9 @@ def adorn_ns( This function adds the original counts (N) to cells that have been converted to percentages. It requires either the original counts to be stored in the DataFrame's attrs (via prior use of adorn_percentages) - or to be passed via the `ns` parameter. + or to be passed via the `ns` parameter. The first column of the counts + is treated as a row identifier and is left untouched, consistent with + `adorn_totals` and `adorn_percentages`. Examples: Add counts to formatted percentages. @@ -366,8 +376,15 @@ def _default_format_func(n): format_func = _default_format_func - # Get numeric columns from the original counts - numeric_cols = ns.select_dtypes(include=[np.number]).columns.tolist() + # Get numeric columns from the original counts, excluding the first + # column which is treated as a row identifier (consistent with R + # janitor's tabyl behavior) + first_col = ns.columns[0] + numeric_cols = [ + col + for col in ns.select_dtypes(include=[np.number]).columns.tolist() + if col != first_col + ] # Apply to matching columns # Use positional indexing to handle cases where df has more rows than ns @@ -470,8 +487,10 @@ def adorn_rounding( ) -> pd.DataFrame: """Round numeric columns with configurable rounding method. - This function rounds all numeric columns to the specified number of - decimal places using the specified rounding method. + This function rounds numeric columns to the specified number of + decimal places using the specified rounding method. The first column is + treated as a row identifier and is left untouched, consistent with + `adorn_totals` and `adorn_percentages`. Examples: Round numeric columns. @@ -523,8 +542,14 @@ def adorn_rounding( # Preserve original counts if they exist original_counts = df.attrs.get("_original_counts") - # Identify numeric columns - numeric_cols = df.select_dtypes(include=[np.number]).columns.tolist() + # Identify numeric columns, excluding the first column which is treated + # as a row identifier (consistent with R janitor's tabyl behavior) + first_col = df.columns[0] + numeric_cols = [ + col + for col in df.select_dtypes(include=[np.number]).columns.tolist() + if col != first_col + ] rounding_mode = ROUND_HALF_EVEN if rounding == "half to even" else ROUND_HALF_UP quantize_str = f"0.{'0' * digits}" if digits > 0 else "0" diff --git a/tests/functions/test_adorn.py b/tests/functions/test_adorn.py index 66351f26b..ac759abc7 100644 --- a/tests/functions/test_adorn.py +++ b/tests/functions/test_adorn.py @@ -391,6 +391,87 @@ def test_adorn_percentages_numeric_first_column(numeric_first_col_df): assert np.isclose(result.iloc[0]["count2"], 5 / 15) +@pytest.mark.functions +def test_adorn_pct_formatting_numeric_first_column(numeric_first_col_df): + """Test adorn_pct_formatting with numeric first column. + + The first column is always treated as a row identifier (consistent with + R janitor tabyl behavior), so it is not formatted as a percentage. + """ + result = numeric_first_col_df.adorn_percentages("row").adorn_pct_formatting() + # First column is treated as row identifier, left as-is + assert result["id"].tolist() == [1, 2, 3] + # The data columns are still formatted + assert result.iloc[0]["count1"] == "66.7%" + assert result.iloc[0]["count2"] == "33.3%" + + +@pytest.mark.functions +def test_adorn_ns_numeric_first_column(numeric_first_col_df): + """Test adorn_ns with numeric first column. + + The first column is always treated as a row identifier (consistent with + R janitor tabyl behavior), so no N count is appended to it. + """ + result = ( + numeric_first_col_df.adorn_percentages("row").adorn_pct_formatting().adorn_ns() + ) + # First column is treated as row identifier, no count appended + assert result["id"].tolist() == [1, 2, 3] + # The data columns still get their counts + assert result.iloc[0]["count1"] == "66.7% (10)" + assert result.iloc[0]["count2"] == "33.3% (5)" + + +@pytest.mark.functions +def test_adorn_ns_numeric_first_column_custom_ns(numeric_first_col_df): + """Test adorn_ns leaves the row identifier alone with an explicit `ns`.""" + custom_ns = pd.DataFrame( + { + "id": [1, 2, 3], + "count1": [100, 200, 300], + "count2": [50, 150, 250], + } + ) + result = ( + numeric_first_col_df.adorn_percentages("row") + .adorn_pct_formatting() + .adorn_ns(ns=custom_ns) + ) + assert result["id"].tolist() == [1, 2, 3] + assert result.iloc[0]["count1"] == "66.7% (100)" + + +@pytest.mark.functions +def test_adorn_rounding_numeric_first_column(numeric_first_col_df): + """Test adorn_rounding with numeric first column. + + The first column is always treated as a row identifier (consistent with + R janitor tabyl behavior), so it is not rounded and keeps its dtype. + """ + result = numeric_first_col_df.adorn_percentages("row").adorn_rounding(digits=2) + # First column is treated as row identifier, values and dtype preserved + assert result["id"].tolist() == [1, 2, 3] + assert result["id"].dtype == numeric_first_col_df["id"].dtype + # The data columns are still rounded + assert result.iloc[0]["count1"] == 0.67 + assert result.iloc[0]["count2"] == 0.33 + + +@pytest.mark.functions +def test_adorn_pipeline_numeric_first_column_from_tabyl(): + """Test the tabyl pipeline keeps a numeric grouping variable readable.""" + df = pd.DataFrame( + { + "cyl": [4, 6, 8, 4, 6, 8, 4, 6], + "gear": [3, 3, 3, 4, 4, 4, 5, 5], + } + ) + result = df.tabyl("cyl", "gear").adorn_percentages("row").adorn_pct_formatting() + # The grouping variable stays a readable row identifier + assert result["cyl"].tolist() == [4, 6, 8] + + # Tests for adorn_ns with custom ns DataFrame From 7e655ce2622eb39c8bb5f5e70135820654129e04 Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Mon, 24 Aug 2026 23:53:17 +0300 Subject: [PATCH 2/3] fix(adorn): select the row identifier by position, from the target frame Addresses review on #1677. adorn_ns took its column list from the `ns` frame and dropped that frame's first column. A counts frame passed through `ns` need not repeat the identifier column of `df`, so a real count in position 0 was discarded with no error. The identifier belongs to the frame being adorned, so it is now read from `df` and every numeric column of `ns` is treated as a count. This matches R janitor's adorn_ns(), which restores column index 1 of `dat` and does not derive its column set from a user-supplied `ns`. The identifier was described as positional but implemented as a label comparison, so a frame with duplicate column labels exempted every column sharing the first column's name. Selection is now positional throughout the adorn_* family, matching R janitor's `setdiff(numeric_cols, 1)`. A DataFrame with no columns raised IndexError on `df.columns[0]`. Such a frame is now returned unchanged. --- CHANGELOG.md | 9 +- janitor/functions/adorn.py | 241 +++++++++++++++++++++------------- tests/functions/test_adorn.py | 137 +++++++++++++++++++ 3 files changed, 295 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 780550a4c..d653d3b74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,14 @@ - [PERF] Reduce peak memory in `_build_indexer_reorder_contents` for wide frames (reps >= 8) using single NumPy allocation; tall frames with few repetitions retain the original reshape path. - Issue #1655 @Anupam2400 - [ENH] `conditional_join` now picks the most selective `<`/`<=`/`>`/`>=` predicate as its binary-search anchor (instead of the first one supplied) when `keep` is `'first'` or `'last'`, fixing pathological slowdowns from unfavorable predicate ordering; the choice is estimated from a fixed-size sample so the selection cost no longer scales with input size; `keep='all'` output is unaffected. - Issue #1641 @samukweku - [BUG] Fix `conditional_join` crash for `keep='last'` joins with a single `<`/`<=` window and multiple non-equi conditions - a missing `counts` argument to the Rust `index_starts_only_keep_last` call. - Issue #1641 @samukweku -- [BUG] `adorn_pct_formatting`, `adorn_ns` and `adorn_rounding` no longer treat a numeric first column as data. The first column is a row identifier, matching `adorn_totals` and `adorn_percentages`. @dylanpulver +- [BUG] `adorn_pct_formatting`, `adorn_ns` and `adorn_rounding` no longer + treat a numeric first column as data. The row identifier is now selected + by column position across the whole `adorn_*` family, so a frame with + duplicate column labels exempts only its first column; `adorn_ns` takes + the identifier from the frame being adorned rather than from `ns`, so a + counts frame that carries no identifier column keeps every count; and a + DataFrame with no columns is returned unchanged instead of raising + `IndexError`. - Issue #1676 @dylanpulver - [ENH] Avoid copying column data during `conditional_join` input validation. - Issue #1645, PR #1642 @samukweku - [ENH] Limit `conditional_join` matching work to columns referenced by join diff --git a/janitor/functions/adorn.py b/janitor/functions/adorn.py index ff3d6aff1..c0e258d2e 100644 --- a/janitor/functions/adorn.py +++ b/janitor/functions/adorn.py @@ -16,6 +16,47 @@ from janitor.utils import check +def _numeric_positions(df: pd.DataFrame) -> list[int]: + """Positions of the numeric columns of a DataFrame. + + Columns are reported by position rather than by label, so that a frame + carrying duplicate column labels is described unambiguously, and a frame + with no columns yields an empty list rather than raising. + + Args: + df: A pandas DataFrame. + + Returns: + A list of integer column positions, in ascending order. + """ + # Relabel a zero-row view by position, so `select_dtypes` reports + # positions instead of labels while keeping its dtype semantics. + probe = df.iloc[:0].set_axis(range(df.shape[1]), axis="columns") + return probe.select_dtypes(include=[np.number]).columns.tolist() + + +def _numeric_data_positions(df: pd.DataFrame) -> list[int]: + """Positions of the numeric columns that the `adorn_*` functions modify. + + Column position 0 holds the row identifier and is never adorned, even + when it is numeric. This mirrors R janitor, whose `adorn_*` functions + drop column index 1 from their numeric column set before adorning + (`numeric_cols <- setdiff(numeric_cols, 1)`). + + The rule is positional, not by label: a frame whose first column label + repeats later on exempts only the first column itself. A frame with no + columns yields an empty list, so callers iterate zero times instead of + indexing into an empty axis. + + Args: + df: A pandas DataFrame. + + Returns: + A list of integer column positions, in ascending order. + """ + return [pos for pos in _numeric_positions(df) if pos != 0] + + @pf.register_dataframe_method def adorn_totals( df: pd.DataFrame, @@ -30,6 +71,12 @@ def adorn_totals( columns. It is particularly useful when working with frequency tables (tabyls) but works on any DataFrame with numeric columns. + The column in position 0 is treated as the row identifier and is never + summed, even when it holds numeric data. This follows R janitor, whose + `adorn_*` functions drop column index 1 before adorning. On a DataFrame + that is not a frequency table, a numeric first column is therefore left + alone. + Examples: Add a totals row to a DataFrame. @@ -76,37 +123,37 @@ def adorn_totals( df = df.copy() + # Nothing to total, and no row identifier to label + if df.shape[1] == 0: + return df + # Store original counts in attrs for adorn_ns to use later if "_original_counts" not in df.attrs: df.attrs["_original_counts"] = df.copy() - # Identify numeric columns, excluding the first column which is treated - # as a row identifier (consistent with R janitor's tabyl behavior) - first_col = df.columns[0] - numeric_cols = [ - col - for col in df.select_dtypes(include=[np.number]).columns.tolist() - if col != first_col - ] + numeric_positions = _numeric_data_positions(df) if where in ("col", "both"): # Add totals column - df[name] = df[numeric_cols].sum(axis=1, skipna=na_rm) + df[name] = df.iloc[:, numeric_positions].sum(axis=1, skipna=na_rm) + # The new totals column is itself summed into the totals row + numeric_positions = _numeric_data_positions(df) if where in ("row", "both"): # Create totals row - totals_row = {} - for col in df.columns: - if col in numeric_cols or col == name: - totals_row[col] = df[col].sum(skipna=na_rm) - elif col == first_col: - # First column gets the totals row name (e.g., "Total") - totals_row[col] = name + numeric = set(numeric_positions) + totals_row = [] + for pos in range(df.shape[1]): + if pos in numeric: + totals_row.append(df.iloc[:, pos].sum(skipna=na_rm)) + elif pos == 0: + # Position 0 gets the totals row name (e.g., "Total") + totals_row.append(name) else: # All other non-numeric columns get the fill value - totals_row[col] = fill + totals_row.append(fill) - totals_df = pd.DataFrame([totals_row]) + totals_df = pd.DataFrame([totals_row], columns=df.columns) df = pd.concat([df, totals_df], ignore_index=True) return df @@ -124,6 +171,12 @@ def adorn_percentages( specified denominator. It is particularly useful after creating frequency tables. + The column in position 0 is treated as the row identifier and is never + modified, even when it holds numeric data. This follows R janitor, whose + `adorn_*` functions drop column index 1 before adorning. On a DataFrame + that is not a frequency table, a numeric first column is therefore left + alone. + Examples: Convert counts to row percentages. @@ -170,32 +223,25 @@ def adorn_percentages( if "_original_counts" not in df.attrs: df.attrs["_original_counts"] = df.copy() - # Identify numeric columns, excluding the first column which is treated - # as a row identifier (consistent with R janitor's tabyl behavior) - first_col = df.columns[0] - numeric_cols = [ - col - for col in df.select_dtypes(include=[np.number]).columns.tolist() - if col != first_col - ] + numeric_positions = _numeric_data_positions(df) - if not numeric_cols: + if not numeric_positions: return df if denominator == "row": - row_totals = df[numeric_cols].sum(axis=1, skipna=na_rm) - for col in numeric_cols: - df[col] = df[col] / row_totals + row_totals = df.iloc[:, numeric_positions].sum(axis=1, skipna=na_rm) + for pos in numeric_positions: + df.isetitem(pos, df.iloc[:, pos] / row_totals) elif denominator == "col": - for col in numeric_cols: - col_total = df[col].sum(skipna=na_rm) + for pos in numeric_positions: + col_total = df.iloc[:, pos].sum(skipna=na_rm) if col_total != 0: - df[col] = df[col] / col_total + df.isetitem(pos, df.iloc[:, pos] / col_total) else: # "all" - grand_total = df[numeric_cols].sum(skipna=na_rm).sum() + grand_total = df.iloc[:, numeric_positions].sum(skipna=na_rm).sum() if grand_total != 0: - for col in numeric_cols: - df[col] = df[col] / grand_total + for pos in numeric_positions: + df.isetitem(pos, df.iloc[:, pos] / grand_total) return df @@ -211,8 +257,12 @@ def adorn_pct_formatting( This function formats numeric columns (assumed to be proportions between 0 and 1) as percentage strings with the specified number of decimal places. - The first column is treated as a row identifier and is left untouched, - consistent with `adorn_totals` and `adorn_percentages`. + + The column in position 0 is treated as the row identifier and is never + formatted, even when it holds numeric data. This follows R janitor, whose + `adorn_*` functions drop column index 1 before adorning. On a DataFrame + that is not a frequency table, a numeric first column is therefore left + alone. Examples: Format percentages with default settings. @@ -265,14 +315,7 @@ def adorn_pct_formatting( # Preserve original counts if they exist original_counts = df.attrs.get("_original_counts") - # Identify numeric columns, excluding the first column which is treated - # as a row identifier (consistent with R janitor's tabyl behavior) - first_col = df.columns[0] - numeric_cols = [ - col - for col in df.select_dtypes(include=[np.number]).columns.tolist() - if col != first_col - ] + numeric_positions = _numeric_data_positions(df) rounding_mode = ROUND_HALF_EVEN if rounding == "half to even" else ROUND_HALF_UP quantize_str = f"0.{'0' * digits}" if digits > 0 else "0" @@ -291,8 +334,8 @@ def format_pct(value): result += "%" return result - for col in numeric_cols: - df[col] = df[col].apply(format_pct) + for pos in numeric_positions: + df.isetitem(pos, df.iloc[:, pos].apply(format_pct)) # Restore original counts in attrs if original_counts is not None: @@ -313,9 +356,15 @@ def adorn_ns( This function adds the original counts (N) to cells that have been converted to percentages. It requires either the original counts to be stored in the DataFrame's attrs (via prior use of adorn_percentages) - or to be passed via the `ns` parameter. The first column of the counts - is treated as a row identifier and is left untouched, consistent with - `adorn_totals` and `adorn_percentages`. + or to be passed via the `ns` parameter. + + The row identifier belongs to the frame being adorned, so it is the + column in position 0 of `df` that is left untouched, even when it holds + numeric data. Every numeric column of `ns` is read as a count, because a + counts frame supplied through `ns` need not repeat the identifier column. + Counts are matched to `df` by column label and to rows by position, so + rows of `df` beyond the end of `ns` (a totals row, for instance) are left + as they are. Examples: Add counts to formatted percentages. @@ -376,33 +425,34 @@ def _default_format_func(n): format_func = _default_format_func - # Get numeric columns from the original counts, excluding the first - # column which is treated as a row identifier (consistent with R - # janitor's tabyl behavior) - first_col = ns.columns[0] - numeric_cols = [ - col - for col in ns.select_dtypes(include=[np.number]).columns.tolist() - if col != first_col - ] - - # Apply to matching columns - # Use positional indexing to handle cases where df has more rows than ns - # (e.g., after adorn_totals adds a totals row) - df_index_list = df.index.tolist() - for col in numeric_cols: - if col in df.columns: - for i, idx in enumerate(df_index_list): - # Only process rows that exist in the original counts - if i < len(ns): - n_value = ns.iloc[i][col] - formatted_n = format_func(n_value) - current_value = df.loc[idx, col] - if pd.notna(current_value) and formatted_n: - if position == "rear": - df.loc[idx, col] = f"{current_value} {formatted_n}" - else: - df.loc[idx, col] = f"{formatted_n} {current_value}" + # Every numeric column of the counts frame is a count. `ns` is not + # required to carry a row identifier, so nothing is dropped from it; + # the identifier is taken from `df` below. + ns_positions = {} + for pos in _numeric_positions(ns): + ns_positions.setdefault(ns.columns[pos], pos) + + # Column position 0 of `df` is the row identifier and is skipped. + # Rows are matched by position, so a `df` with more rows than `ns` + # (e.g., after adorn_totals adds a totals row) leaves the extras alone. + n_rows = min(len(df), len(ns)) + for pos in range(1, df.shape[1]): + ns_pos = ns_positions.get(df.columns[pos]) + if ns_pos is None: + continue + values = list(df.iloc[:, pos]) + adorned = False + for i in range(n_rows): + formatted_n = format_func(ns.iat[i, ns_pos]) + current_value = values[i] + if pd.notna(current_value) and formatted_n: + if position == "rear": + values[i] = f"{current_value} {formatted_n}" + else: + values[i] = f"{formatted_n} {current_value}" + adorned = True + if adorned: + df.isetitem(pos, values) return df @@ -457,6 +507,10 @@ def adorn_title( df = df.copy() + # No columns means no row or column variable to name + if df.shape[1] == 0: + return df + first_col = df.columns[0] if row_name is None: row_name = str(first_col) @@ -488,9 +542,13 @@ def adorn_rounding( """Round numeric columns with configurable rounding method. This function rounds numeric columns to the specified number of - decimal places using the specified rounding method. The first column is - treated as a row identifier and is left untouched, consistent with - `adorn_totals` and `adorn_percentages`. + decimal places using the specified rounding method. + + The column in position 0 is treated as the row identifier and is never + rounded, even when it holds numeric data. This follows R janitor, whose + `adorn_*` functions drop column index 1 before adorning. On a DataFrame + that is not a frequency table, a numeric first column is therefore left + alone. Examples: Round numeric columns. @@ -516,6 +574,14 @@ def adorn_rounding( 0 A 1.2 3.5 1 B 2.6 4.6 + A numeric first column is a row identifier, so it is not rounded. + + >>> ordinary = pd.DataFrame({"year": [2020.4, 2021.6], "value": [1.234, 2.345]}) + >>> ordinary.adorn_rounding(digits=1) + year value + 0 2020.4 1.2 + 1 2021.6 2.3 + Args: df: A pandas DataFrame. digits: Number of decimal places to round to. @@ -542,14 +608,7 @@ def adorn_rounding( # Preserve original counts if they exist original_counts = df.attrs.get("_original_counts") - # Identify numeric columns, excluding the first column which is treated - # as a row identifier (consistent with R janitor's tabyl behavior) - first_col = df.columns[0] - numeric_cols = [ - col - for col in df.select_dtypes(include=[np.number]).columns.tolist() - if col != first_col - ] + numeric_positions = _numeric_data_positions(df) rounding_mode = ROUND_HALF_EVEN if rounding == "half to even" else ROUND_HALF_UP quantize_str = f"0.{'0' * digits}" if digits > 0 else "0" @@ -562,8 +621,8 @@ def round_value(value): ) return float(rounded) - for col in numeric_cols: - df[col] = df[col].apply(round_value) + for pos in numeric_positions: + df.isetitem(pos, df.iloc[:, pos].apply(round_value)) # Restore original counts in attrs if original_counts is not None: diff --git a/tests/functions/test_adorn.py b/tests/functions/test_adorn.py index ac759abc7..72ab630c0 100644 --- a/tests/functions/test_adorn.py +++ b/tests/functions/test_adorn.py @@ -569,3 +569,140 @@ def test_adorn_ns_thousand_separator(): # Should have thousand separator assert "(1,000)" in result.iloc[0]["count"] assert "(2,000,000)" in result.iloc[1]["count"] + + +# Tests for review findings on PR #1677: the row identifier is positional, +# belongs to the frame being adorned, and must not break empty frames. + + +@pytest.mark.functions +def test_adorn_ns_counts_frame_without_identifier_column(): + """A counts frame that carries no row identifier keeps every count. + + `ns` is a separate frame and is not required to repeat the identifier + column of `df`. Reading the identifier off `ns` drops the count in its + position 0 with no error raised. + """ + df = pd.DataFrame({"id": ["A"], "yes": [0.5], "no": [0.5]}) + ns = pd.DataFrame({"yes": [10], "no": [5]}) + + result = df.adorn_pct_formatting().adorn_ns(ns=ns) + + # Detector: position 0 of `ns` is a count, not an identifier + assert result.iloc[0]["yes"] == "50.0% (10)" + # Pins: unchanged by the fix + assert result.iloc[0]["no"] == "50.0% (5)" + assert result.iloc[0]["id"] == "A" + + +@pytest.mark.functions +def test_adorn_ns_identifier_comes_from_target_frame(): + """The identifier is position 0 of `df`, whatever `ns` looks like. + + The counts frame here shares its labels with `df` but is ordered + differently, so a rule derived from `ns` would exempt the wrong column. + """ + df = pd.DataFrame({"id": ["A"], "yes": [0.5], "no": [0.5]}) + ns = pd.DataFrame({"no": [5], "yes": [10], "id": [0]}) + + result = df.adorn_pct_formatting().adorn_ns(ns=ns) + + # Detector: `no` sits at position 0 of `ns` but is data in `df` + assert result.iloc[0]["no"] == "50.0% (5)" + # Detector: `id` is numeric in `ns` but is the identifier of `df` + assert result.iloc[0]["id"] == "A" + # Pin + assert result.iloc[0]["yes"] == "50.0% (10)" + + +@pytest.mark.functions +@pytest.mark.parametrize( + "call", + [ + lambda df: df.adorn_pct_formatting(), + lambda df: df.adorn_rounding(), + lambda df: df.adorn_ns(ns=pd.DataFrame()), + lambda df: df.adorn_totals(), + lambda df: df.adorn_percentages(), + lambda df: df.adorn_title(), + ], + ids=[ + "pct_formatting", + "rounding", + "ns", + "totals", + "percentages", + "title", + ], +) +def test_adorn_on_dataframe_with_no_columns(call): + """A DataFrame with no columns comes back unchanged, not an IndexError.""" + result = call(pd.DataFrame()) + + assert result.shape == (0, 0) + assert list(result.columns) == [] + + +@pytest.mark.functions +def test_adorn_pct_formatting_duplicate_column_labels(): + """Only column position 0 is exempt when labels repeat. + + A label based rule exempts every column sharing the first column's name, + which silently leaves data columns unformatted. + """ + df = pd.DataFrame([[1.0, 2.0, 0.5]], columns=["value", "value", "ratio"]) + + result = df.adorn_pct_formatting() + + # Detector: the second `value` column is data and is formatted. + # Its dtype flips from float to str, so this cannot pass both ways. + assert result.iloc[0, 1] == "200.0%" + assert isinstance(result.iloc[0, 1], str) + # Detector: position 0 keeps its numeric value and is not formatted + assert result.iloc[0, 0] == 1.0 + assert not isinstance(result.iloc[0, 0], str) + # Pin + assert result.iloc[0, 2] == "50.0%" + + +@pytest.mark.functions +def test_adorn_rounding_duplicate_column_labels(): + """Rounding also exempts only column position 0 when labels repeat.""" + df = pd.DataFrame([[1.25, 2.55, 0.55]], columns=["value", "value", "ratio"]) + + result = df.adorn_rounding(digits=1, rounding="half up") + + # Detector: the second `value` column is rounded + assert result.iloc[0, 1] == 2.6 + # Detector: position 0 is left at full precision + assert result.iloc[0, 0] == 1.25 + # Pin + assert result.iloc[0, 2] == 0.6 + + +@pytest.mark.functions +def test_adorn_totals_duplicate_column_labels(): + """The totals row sums every numeric column except position 0.""" + df = pd.DataFrame( + [[1.0, 2.0, 3.0], [4.0, 5.0, 6.0]], + columns=["value", "value", "ratio"], + ) + + result = df.adorn_totals("row") + + # Detector: the second `value` column is summed + assert result.iloc[-1, 1] == 7.0 + # Pins + assert result.iloc[-1, 0] == "Total" + assert result.iloc[-1, 2] == 9.0 + + +@pytest.mark.functions +def test_adorn_rounding_leaves_numeric_first_column_of_plain_dataframe(): + """The identifier rule applies to any DataFrame, not only to tabyls.""" + df = pd.DataFrame({"year": [2020.4, 2021.6], "value": [1.234, 2.345]}) + + result = df.adorn_rounding(digits=1) + + assert result["year"].tolist() == [2020.4, 2021.6] + assert result["value"].tolist() == [1.2, 2.3] From 57a8de4b8793a4c06e576bb25dab79a62ce5453b Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Wed, 26 Aug 2026 12:44:00 +0300 Subject: [PATCH 3/3] fix(adorn): match counts to duplicate labels one occurrence at a time adorn_ns keyed its count lookup by label, keeping a single position per label, so every column of df sharing a label read the same count. Counts that share the column axis of df are now matched across by position (the stored-counts path, where the identifier would otherwise stand in as a count), and a label supplied through ns is otherwise consumed occurrence by occurrence. --- CHANGELOG.md | 9 ++++--- janitor/functions/adorn.py | 48 ++++++++++++++++++++++++++++++----- tests/functions/test_adorn.py | 43 +++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d653d3b74..656b339ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,12 @@ by column position across the whole `adorn_*` family, so a frame with duplicate column labels exempts only its first column; `adorn_ns` takes the identifier from the frame being adorned rather than from `ns`, so a - counts frame that carries no identifier column keeps every count; and a - DataFrame with no columns is returned unchanged instead of raising - `IndexError`. - Issue #1676 @dylanpulver + counts frame that carries no identifier column keeps every count; a + counts frame sharing the column axis of the frame being adorned is + matched across by position, and a repeated label is otherwise matched + occurrence by occurrence, so duplicate labels no longer all read the + first count; and a DataFrame with no columns is returned unchanged + instead of raising `IndexError`. - Issue #1676 @dylanpulver - [ENH] Avoid copying column data during `conditional_join` input validation. - Issue #1645, PR #1642 @samukweku - [ENH] Limit `conditional_join` matching work to columns referenced by join diff --git a/janitor/functions/adorn.py b/janitor/functions/adorn.py index c0e258d2e..14a4eab18 100644 --- a/janitor/functions/adorn.py +++ b/janitor/functions/adorn.py @@ -362,9 +362,13 @@ def adorn_ns( column in position 0 of `df` that is left untouched, even when it holds numeric data. Every numeric column of `ns` is read as a count, because a counts frame supplied through `ns` need not repeat the identifier column. - Counts are matched to `df` by column label and to rows by position, so - rows of `df` beyond the end of `ns` (a totals row, for instance) are left - as they are. + + Counts are matched to rows by position, so rows of `df` beyond the end of + `ns` (a totals row, for instance) are left as they are. Columns are + matched by position when `ns` shares the column axis of `df` -- the case + for counts stored by `adorn_percentages` -- and by label otherwise. A + label that repeats is matched occurrence by occurrence, so the k-th + column of `df` carrying a label reads the k-th count column carrying it. Examples: Add counts to formatted percentages. @@ -428,16 +432,46 @@ def _default_format_func(n): # Every numeric column of the counts frame is a count. `ns` is not # required to carry a row identifier, so nothing is dropped from it; # the identifier is taken from `df` below. - ns_positions = {} - for pos in _numeric_positions(ns): - ns_positions.setdefault(ns.columns[pos], pos) + ns_numeric = _numeric_positions(ns) + + # A counts frame that shares its column axis with `df` describes the very + # same columns, so it is matched straight across by position. This is the + # stored-counts path (`_original_counts` is a copy of the frame that + # `adorn_percentages` was handed), and matching it by label would be + # ambiguous the moment a label repeats. + aligned = df.columns.equals(ns.columns) + if aligned: + ns_numeric_set = set(ns_numeric) + + def _count_position(pos: int, label) -> Optional[int]: + return pos if pos in ns_numeric_set else None + + else: + # Otherwise counts are matched by label. A label that repeats in `ns` + # is consumed occurrence by occurrence, so the k-th column of `df` + # carrying a label reads the k-th count column carrying it, rather + # than every one of them re-reading the first. + ns_by_label: dict = {} + for ns_pos in ns_numeric: + ns_by_label.setdefault(ns.columns[ns_pos], []).append(ns_pos) + seen: dict = {} + + def _count_position(pos: int, label) -> Optional[int]: + candidates = ns_by_label.get(label) + if candidates is None: + return None + occurrence = seen.get(label, 0) + seen[label] = occurrence + 1 + if occurrence >= len(candidates): + return None + return candidates[occurrence] # Column position 0 of `df` is the row identifier and is skipped. # Rows are matched by position, so a `df` with more rows than `ns` # (e.g., after adorn_totals adds a totals row) leaves the extras alone. n_rows = min(len(df), len(ns)) for pos in range(1, df.shape[1]): - ns_pos = ns_positions.get(df.columns[pos]) + ns_pos = _count_position(pos, df.columns[pos]) if ns_pos is None: continue values = list(df.iloc[:, pos]) diff --git a/tests/functions/test_adorn.py b/tests/functions/test_adorn.py index 72ab630c0..79260772a 100644 --- a/tests/functions/test_adorn.py +++ b/tests/functions/test_adorn.py @@ -706,3 +706,46 @@ def test_adorn_rounding_leaves_numeric_first_column_of_plain_dataframe(): assert result["year"].tolist() == [2020.4, 2021.6] assert result["value"].tolist() == [1.2, 2.3] + + +@pytest.mark.functions +def test_adorn_ns_duplicate_labels_in_explicit_ns(): + """Each repeat of a label reads its own count column. + + A label keyed lookup keeps only one position per label, so every `yes` + column of `df` would read the first `yes` count and the second count + would be silently dropped. + """ + df = pd.DataFrame( + [["A", "50.0%", "50.0%"]], + columns=["id", "yes", "yes"], + ) + ns = pd.DataFrame([[10, 20]], columns=["yes", "yes"]) + + result = df.adorn_ns(ns=ns) + + # Detector: the second `yes` reads the second count, not the first + assert result.iloc[0, 2] == "50.0% (20)" + # Pins + assert result.iloc[0, 1] == "50.0% (10)" + assert result.iloc[0, 0] == "A" + + +@pytest.mark.functions +def test_adorn_ns_duplicate_labels_in_stored_counts_pipeline(): + """Stored counts are matched by position when they share the axis. + + The counts frame kept by `adorn_percentages` is a copy of the frame it + was handed, identifier column and all. Matching it by label would let + the identifier stand in as a count for every data column. + """ + df = pd.DataFrame([[100, 10, 20]], columns=["x", "x", "x"]) + + result = df.adorn_percentages("row").adorn_pct_formatting().adorn_ns() + + # Detectors: each data column reads its own count, and neither reads + # the identifier in position 0. + assert result.iloc[0, 1] == "33.3% (10)" + assert result.iloc[0, 2] == "66.7% (20)" + # Pin: the identifier keeps its value and is not adorned + assert result.iloc[0, 0] == 100