[TEST] Adding Sparse Triangle to Unittest - #1157
Conversation
|
@genedan can you take care of the 6 failing tests in test_slicing? |
Pyright Type CompletenessView the full Project (full
Other symbols referenced but not exported by
Symbols without documentation:
Patch (exported symbols added or changed by this PR): 28.6% fully typed (2 / 7); 2 no longer exported
Patch symbol details
|
* fixing fixture expecting errors * fixing prism * trying another approach * some fixes * more fixes * more fixes * fix for learning * more learning fix * restoring test still fails
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7c8e5fe. Configure here.
|
@genedan there are two more tests in test_triangle that you authored. i'm not sure if those tests should be sparse-compatible. could you please take a look? |
* further improving prism_convert * capecod & mack fixes * fixing test_utilities and removing sparse.testing * more fixes * implementing nanquantile and nanmedian * improving comment, adding tests, fixing bugbot * fixing test * removing empty check
* Release prep * Updated release notes * feat(core): add index/columns/origin/development alternatives to Triangle.drop() Route the keyword alternatives to their axis, mirroring pandas. Refs #1060. * docs(core): add drop() examples and wrap TriangleProtocol.drop signature Address review on #1131: format the long drop() signature one arg per line and add testable Examples to the docstring. * fix(core): handle integer labels in Triangle.drop() Normalize any scalar (str or int) into a list so a bare int routes to its axis instead of raising TypeError. Addresses Bugbot review on #1131. * fix(core): preserve list-like handling in Triangle.drop() Use np.isscalar so pd.Index/ndarray labels pass through list() while scalars (str/int) are wrapped, and a bare drop() still raises. Addresses Bugbot review on #1131. * Sync from main, added a few more updates * docs(core): show original columns in drop() example Print the triangle's original columns before the drop, per review on #1131. * Added release date * Bump uv * Apply lognormal fix for apriori draws and add tests * Added sphinx-book-theme dependency * Added contributors full names * Added contributor fullnames and missing tickets * Tighten sphinx-book-theme version * bug bot fix * Fix documentation build warnings and cross-references * Restore local Sphinx extension path * Fix Development reference in user guide * Restore user guide section links. * Fix Triangle display crash when stored in a DataFrame cell (GH #142) A Triangle is a 4-D container but pandas' is_sequence() misclassified it as a 1-D sequence because the legacy __getitem__ iteration protocol plus __len__ made iter() succeed. Formatting a DataFrame that holds a Triangle in a cell then iterated the Triangle and raised, crashing both the text and HTML reprs. Declare Triangle non-iterable via __iter__ = None so pandas falls back to str(triangle) and renders the summary. Add regression tests. * Remove dead backend-reset code in _prep_columns (#1045) * FEAT: Explicitly forbid in-place assignment on sparse array. * FIX: Fix bug in column assignment with a sparse backend. * FEAT: Enable Triangle __array__ for sparse backend. * TEST: Fix tests. * FIX: Apply bugbot fix. * TST: Add concurrency guard. --------- Co-authored-by: Kenneth Hsu <[email protected]> Co-authored-by: priyam0k <[email protected]> Co-authored-by: Howard Friman <[email protected]> Co-authored-by: Anonymous <[email protected]> Co-authored-by: Gene Dan <[email protected]>
updating again
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1157 +/- ##
==========================================
- Coverage 91.27% 91.14% -0.13%
==========================================
Files 91 91
Lines 5422 5365 -57
Branches 691 681 -10
==========================================
- Hits 4949 4890 -59
- Misses 339 340 +1
- Partials 134 135 +1
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:
|
genedan
left a comment
There was a problem hiding this comment.
Minor comments/questions and nitpicks on spacing.
| origins = X.age_to_age.values.shape[2] | ||
| reg_x = X.incr_to_cum().values[...,:origins,:-1] | ||
| reg_y = X.incr_to_cum().values[...,:origins,1:] | ||
| #following precedent set by _set_fit_groups() from DevelopmentBase to force triangle to be dense |
There was a problem hiding this comment.
Would there be a use case where we'd need the triangle to sparse?
There was a problem hiding this comment.
no, this was one of the main points behind why i think our long term strategy for the package is to abandon sparse altogether. sparse has never been used in estimating parameters, because there is zero point of doing so. a sparse triangle is by default a single claim. the age_to_age on a single claim is meaningless. ldf_ is always dense.
| prism : Triangle | ||
| The prism sample data set Triangle. | ||
| prism_convert : Triangle | ||
| The prism sample data set Triangle, reduced for test speed. |
There was a problem hiding this comment.
It makes sense from this docstring why the new fixture was created, a comment in the corresponding definition in conftest.py would be helpful.
There was a problem hiding this comment.
i'm actually gonna get rid of prism_convert, and follow your logic from test_auto_sparse_converts_numpy_to_sparse
|
|
||
|
|
||
| def nan_to_num(a): | ||
| def nan_to_num(a,nan=0.0): |
| out : COO | ||
| result cast back to COO for consistency. | ||
| """ | ||
| return nanquantile(a,0.5,axis,keepdims) |
|
|
||
| return COO(out) | ||
|
|
||
| def nanmedian(a:COO, axis:int=0, keepdims:bool=False): |
There was a problem hiding this comment.
spaces after the colons and around the equal signs, e.g., keepdims: bool = False
| yield from _sample_fixture(request, "prism") | ||
|
|
||
| @pytest.fixture | ||
| def prism_convert(request): |
There was a problem hiding this comment.
Now that we have 3 different prism fixtures, I wonder if we can adjust the naming convention to prevent confusion and let people know the differences between them. Was prism_convert created as a version that would auto-sparsify, whereas prism_dense is one that always stays dense?
If so, maybe something like:
prism_auto_sparse(A dense version of prism above 30 MB that will auto sparsify after aggregation)prism_always_dense(A dense version of prism below 30 MB that will always stay dense)
With accompanying docstrings to explain their creation - i.e., we have to do some amount of truncation or aggregation at definition time to prevent the fixture from eating up all the RAM.
There was a problem hiding this comment.
prism_dense is actually only used for its monthly grain. renaming it to monthly instead.
…nladder-python into sparse_fixture
Sparse fixture
|
@genedan thanks for the review. i tried to address all comments. |

Summary of Changes
Fixing test fixture to supply actual sparse triangles
sped up numpy conversion of prism in
test_triangleresolved failing tests in
test_developmentadded numpy conversion in
learningto satisfy skl requirement that sample_weight is denseresolved failing test in
cape_codadded backend conversion in
mackfor sparse compatibilityadded nanquantile and nanmedian to
sparseadded new tests for nanquantile
added nan to nan_to_num in
sparseRelated GitHub Issue(s)
fixes #1155
Additional Context for Reviewers
uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)Note
Medium Risk
Changes Mack reserve uncertainty recursion and sparse reduction helpers used across triangles; scope is mostly backend alignment and test coverage, but Mack and sparse quantile paths affect numerical results on sparse data.
Overview
This PR fixes sparse-backed triangle testing (issue #1155) by changing how fixtures supply backends and aligning code paths when sparse triangles meet numpy-only intermediates.
Test infrastructure:
conftestno longer toggles globalARRAY_BACKEND; fixtures yield triangles viaset_backend("sparse")or numpy per parametrization. The summed-prism fixture is renamedmonthly;prismtests default to sparse-only runs.Sparse utilities: Adds
nanquantile/nanmedianfor COO arrays and extendsnan_to_numwith a configurable NaN replacement—supporting quantile/median-style ops on sparse data.Runtime fixes:
DevelopmentML._prep_w_mlforces dense numpy/cupy weights for sklearn.MackChainladderre-appliesset_backendbefore sums andsummary_concatenation so auto-sparse conversions don’t mix backends. Development test helpers densify sparse triangles before weighted regression.Tests: Assertions increasingly use Triangle equality instead of raw array module tests; grain/auto_sparse tests are adjusted for real sparse inputs and a smaller prism slice for numpy auto_sparse checks.
Reviewed by Cursor Bugbot for commit 9445d09. Bugbot is set up for automated code reviews on this repo. Configure here.