Improve test suite: ridgeplot._hist - #365
Conversation
Signed-off-by: Tomas Pereira de Vasconcelos <tomasvasconcelos1@gmail.com>
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #365 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 19 19
Lines 674 678 +4
Branches 88 88
=========================================
+ Hits 674 678 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39a77d3b32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @pytest.mark.parametrize("non_finite", NON_FINITE_VALUES) | ||
| def test_rejects_non_finite_samples(non_finite: float) -> None: | ||
| with pytest.raises(ValueError, match="samples array should not contain any infs or NaNs"): | ||
| bin_trace_samples([1, 2, non_finite], nbins=2) |
There was a problem hiding this comment.
The new non-finite-value cases duplicate rather than replace the existing tests at lines 71–74 and 172–180, while this patch deletes SAMPLES_IN, NBINS, and WEIGHTS. Consequently, collecting this file runs eight cases that raise NameError before reaching bin_trace_samples; the targeted pytest suite reports 8 failures. Delete those stale tests or make them self-contained so the unit suite passes.
AGENTS.md reference: AGENTS.md:L163-L163
Useful? React with 👍 / 👎.
1 participant
Summary
Rewrites
tests/unit/test_hist.pywith a more comprehensive, better-organised test suite forridgeplot._hist, and fixes the user-facing validation gaps surfaced by the behavioural review of those tests.Test suite
SAMPLES_IN,X_OUT, ...) with parametrised, self-contained test cases: basic binning, single bin, uniform distribution, all-identical samples (zero-width range), and negative valuesestimate_density_trace()/estimate_densities()(copied over fromtest_kde.py)bin_samples()withsample_weights: flat-vector broadcast to all traces, and per-row weights in the shallow form (incl. broadcast to all traces within a multi-trace row)bin_samples()floats (np.float64is a subclass offloat, so a plainisinstance()check wouldn't cut it)-inf) and mismatched weights lengths are parametrised and deduplicated via a sharedNON_FINITE_VALUESconstantBug fixes (validation gaps found while reviewing the tests)
ValueErrorin both the histogram and KDE paths (previously: silent zero-count bins over numpy's default[0, 1]range, followed by a bareZeroDivisionErrorfromridgeplot())sample_weightsnow raise clearValueErrors (previously: opaque crashes deep in the color interpolation logic)zip()errorDocumentation
sample_weightsparameter docs: no longer described as "KDE weights" (it applies to histogram binning too), and the three accepted forms (flat broadcast vector, shallow per-row list, full nested per-trace array) are now documentedRelated
397(support for custom bin edges) as a follow-up feature request