From f7f6233c377c5b374f5c79df0af249ea6d4a8b52 Mon Sep 17 00:00:00 2001 From: Chris Nicholas <4948774+cnicholas@users.noreply.github.com> Date: Tue, 15 Sep 2026 07:12:51 -0400 Subject: [PATCH] fix(derivations): one bin per distinct value when more bins are requested than values exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - **What:** when n >= number of distinct finite values, fit edges at the midpoints between neighbouring values (one bin per value) instead of capping n and re-taking quantiles. - **Why:** 0.3.1's cap collapsed heavily tied columns — 3,000 ones and 1,000 twos with n=4 fitted ONE bin (0.3.0 fitted two) because the halved quantile positions both landed on 1. Caught by the app's preview test on the pin bump. - **Scope:** derivations._fit_edges; two existing tests re-pinned to the n < distinct case they were written to prove; robustness tests extended; CHANGELOG. ## Contract / Invariants (must remain true) - n < number of distinct values: byte-identical edges (original quantile / linspace path; pinned by test_fewer_bins_than_distinct_values_is_untouched and the bin-extremes suite). - Every finite value lands in a bin (fuzz + bin-extremes). - validator exit 0. ## Behavior Changes (explicit) - n >= distinct count: edges = [v1, midpoints..., vk]; message "requested n bins, ties produced k (one per distinct value)". Labels change from interpolated quantile edges (e.g. '[1, 1.25)') to midpoint edges ('[1, 1.5)'); membership is one value per bin. ## Tests - test_heavily_tied_column_keeps_one_bin_per_distinct_value (the regression) - test_more_bins_than_distinct_values_gives_one_bin_per_value (edges + counts) - test_fewer_bins_than_distinct_values_is_untouched - test_tie_drop_uses_fitted_count_labels_and_message and test_validate_label_count_against_fitted_not_requested now request n=4 on 5 distinct values so the quantile-collision case they document still occurs. ## Manual Verification - pytest tests/: 2431 passed; ruff clean; validator exit 0 --- CHANGELOG.md | 8 ++++++++ processbehavior/derivations.py | 19 ++++++++++++------- tests/test_derivations.py | 19 +++++++++++++------ tests/test_derivations_robustness.py | 18 ++++++++++++++++-- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6c07df..fbd4d4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **Binning a heavily tied column no longer collapses to one bin.** 0.3.1 capped the + requested bin count at the number of distinct values *before* taking the quantiles, and on + a column of 3,000 ones and 1,000 twos the halved quantile positions both landed on 1, so + one bin fitted where 0.3.0 fitted two. When at least as many bins are requested as there + are distinct values, the fit now cuts at the midpoints between neighbouring values: one + bin per distinct value, every value in its own bin, with the message ``requested n bins, + ties produced k (one per distinct value)``. Requesting fewer bins than distinct values is + untouched (the original quantile / equal-width path). Found by the app's preview test. - **The connecting line on a lane chart no longer crosses lane boundaries.** On an X or mR chart with factors and ``by=[]`` (one chart, subgroups side by side), the last point of each subgroup was joined to the first point of the next, so every boundary drew a steep diff --git a/processbehavior/derivations.py b/processbehavior/derivations.py index 337bdba..08e006a 100644 --- a/processbehavior/derivations.py +++ b/processbehavior/derivations.py @@ -528,19 +528,24 @@ def _fit_edges(spec: Derivation, present_vals: pd.Series): edges = [-math.inf, mu - 2 * sigma, mu - sigma, mu + sigma, mu + 2 * sigma, math.inf] else: n = params['n'] - n_distinct = int(present_vals.nunique()) - if n_distinct and n > n_distinct: - # More bins than distinct values can only produce empty bins. - message = f'requested {n} bins, only {n_distinct} distinct values; fitted {n_distinct}' - n = n_distinct - if method == 'equal_width': + distinct = np.unique(present_vals.to_numpy(dtype=float)) + if len(distinct) >= 2 and n >= len(distinct): + # More bins than distinct values: one bin per distinct value, cut at the + # midpoints between neighbours. Capping n and re-taking quantiles instead + # collapsed heavily tied columns (3,000 ones and 1,000 twos fitted one bin); + # the requested-n quantiles interpolated between the values and left mostly + # empty bins when n was large. + mids = (distinct[:-1] + distinct[1:]) / 2.0 + edges = [float(distinct[0]), *[float(m) for m in mids], float(distinct[-1])] + message = f'requested {n} bins, ties produced {len(distinct)} (one per distinct value)' + elif method == 'equal_width': lo, hi = float(present_vals.min()), float(present_vals.max()) edges = list(np.linspace(lo, hi, n + 1)) else: # equal_freq qs = np.linspace(0.0, 1.0, n + 1) edges = list(np.unique(np.quantile(present_vals, qs))) if len(edges) - 1 != n: - message = f'requested {params["n"]} bins, ties produced {len(edges) - 1}' + message = f'requested {n} bins, ties produced {len(edges) - 1}' return edges, message diff --git a/tests/test_derivations.py b/tests/test_derivations.py index c429b0b..1456054 100644 --- a/tests/test_derivations.py +++ b/tests/test_derivations.py @@ -133,11 +133,18 @@ def test_equal_freq_labels_and_fitted_edges(): def test_tie_drop_uses_fitted_count_labels_and_message(): + """Fewer bins than distinct values, on a tied column: the quantile edges collide. + + Requesting 4 bins on six 1s and one each of 2..5 puts three quartile edges on 1, so + only 2 bins fit. Labels are keyed on that fitted count, and the message says so. + (Requesting *at least* as many bins as distinct values now fits one per value; see + test_derivations_robustness.) + """ tie = pd.Series([1, 1, 1, 1, 1, 1, 2, 3, 4, 5], dtype=float) - r = evaluate(Derivation.bin('t', n=5, bin_labels='ordinal'), tie) - assert r.fitted['n_bins'] == 3 # qcut dropped duplicate edges - assert r.fitted['labels'] == ['Low', 'Medium', 'High'] # keyed on FITTED count, not 5 - assert 'requested 5 bins' in r.message and 'produced 3' in r.message + r = evaluate(Derivation.bin('t', n=4, bin_labels='ordinal'), tie) + assert r.fitted['n_bins'] == 2 # quantile edges collided on the tied value + assert r.fitted['labels'] == ['Low', 'High'] # keyed on FITTED count, not 4 + assert 'requested 4 bins' in r.message and 'produced 2' in r.message @pytest.mark.parametrize('method', ['equal_freq', 'equal_width', 'sd']) @@ -252,8 +259,8 @@ def test_validate_structured_results(): def test_validate_label_count_against_fitted_not_requested(): df = pd.DataFrame({'t': [1, 1, 1, 1, 1, 1, 2, 3, 4, 5]}) - # request 5 bins with 5 labels, but ties produce 3 bins -> mismatch caught - res = validate(Derivation.bin('t', n=5, bin_labels=['a', 'b', 'c', 'd', 'e']), df) + # request 4 bins with 4 labels, but ties produce 2 bins -> mismatch caught + res = validate(Derivation.bin('t', n=4, bin_labels=['a', 'b', 'c', 'd']), df) assert res.ok is False assert any(i['code'] == 'label_count' for i in res.issues) diff --git a/tests/test_derivations_robustness.py b/tests/test_derivations_robustness.py index e32141c..9f20335 100644 --- a/tests/test_derivations_robustness.py +++ b/tests/test_derivations_robustness.py @@ -183,11 +183,25 @@ def test_near_constant_sd_bins_do_not_raise(self): r = evaluate(B('x', method='sd'), pd.Series([-0.085, -0.085, -0.085, -0.085000001])) assert r.fitted['n_bins'] == 5 and r.values.notna().all() - def test_more_bins_than_distinct_values_is_capped_with_a_message(self): + def test_more_bins_than_distinct_values_gives_one_bin_per_value(self): r = evaluate(B('x', n=1000), pd.Series(np.arange(1.0, 21.0))) - assert r.fitted['n_bins'] == 20 and 'only 20 distinct values' in r.message + assert r.fitted['n_bins'] == 20 and 'ties produced 20 (one per distinct value)' in r.message + assert r.values.value_counts(sort=False).tolist() == [1] * 20 r = evaluate(B('x', method='equal_width', n=9), pd.Series([1.0, 2.0, 3.0])) assert r.fitted['n_bins'] == 3 and r.values.notna().all() + assert r.fitted['edges'] == [1.0, 1.5, 2.5, 3.0] + + def test_heavily_tied_column_keeps_one_bin_per_distinct_value(self): + """Regression: capping n before the quantiles collapsed 3,000 ones + 1,000 twos to one bin.""" + tied = pd.Series([1.0] * 3000 + [2.0] * 1000) + r = evaluate(B('x', n=4), tied) + assert r.fitted['n_bins'] == 2 and 'ties' in r.message + assert r.values.value_counts(sort=False).tolist() == [3000, 1000] + + def test_fewer_bins_than_distinct_values_is_untouched(self): + """n below the distinct count takes the original quantile / linspace path.""" + r = evaluate(B('x', n=4), pd.Series(np.arange(1.0, 21.0))) + assert r.fitted['edges'] == [1.0, 5.75, 10.5, 15.25, 20.0] and r.message is None def test_ordinal_beyond_five_bins_says_so(self): r = evaluate(B('x', n=6, bin_labels='ordinal'), pd.Series(np.arange(1.0, 61.0)))