Add TriangleWeight utility class

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready

[henrydingliu]

Summary of Changes

Adding TriangleWeight utility class as a public API into the "latest_n" and various drop weight assigning logic behind the Development estimator. Mostly copy/paste from DevelopmentBase, with cleanup and additional comments (thanks to @kennethshsu for adding the original comments).

Note that what's copied over are the newer drop functions (with the _func) suffix. more history on those below

Top half of Test_development is refactored to show parity between TriangleWeight and the original drop logic in DevelopmentBase. They add on top of the series of new_drop tests that cover the same, newer drop functions.

Adding import TriangleWeight to various places

Redirecting other children of DevelopmentBase to use TriangleWeight instead of _set_weight_func.

Deleting _set_weight_func and all associated private functions from DevelopmentBase

Tweaking tests as needed

Related GitHub Issue(s)

closes#932

Additional Context for Reviewers

Development currently runs on a series of robust internal functions for handling various drop parameters. These functions are truly the backbone of the package. A couple of years back, I needed to do some convoluted dropping around IncrementalAdditive, and found that the dropping logic in IncrementalAdditive was implemented separately and was less robust. The separate implementation was due to the dropping functions in DevelopmentBase all assuming that the weights are being assigned to age-to-age factors (i.e. one less period than the underlying loss or count triangle). In an effort to consolidate the dropping logic, I copied each of the original drop functions and made a slightly more generalized version. Everything from here on are all copied and tweaked from John's original code. You can tell them apart based on the _func suffix. These tweaked copies now power the dropping logic in IncrementalAdditive. But we never got around to shifting the rest of DevelopmentBase over.

• I passed tests locally for both code (uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)

---

Note

Medium Risk
Refactors core actuarial drop/weight logic used for LDFs across multiple estimators; behavior is heavily regression-tested but any subtle divergence from removed DevelopmentBase helpers could change reserves.

Overview
Introduces a public TriangleWeight sklearn-style utility that builds triangle weights from n_periods, cell/valuation drops, rank trims (drop_high/drop_low), and threshold filters (drop_above/drop_below), with preserve semantics and the same exclusion warnings the tests assert on.

DevelopmentBase drops the duplicated _*_func weight helpers (~270 lines); Development, IncrementalAdditive, and DevelopmentML now call TriangleWeight.fit(...).w_ instead of _set_weight_func. Development still computes regression w_ via the older _assign_n_periods_weight / _drop_adjustment path while w_v2_ is populated through TriangleWeight on age-to-age factors (including pipeline parity tests on w_v2_).

TriangleWeight is exported from chainladder and chainladder.utils; test_development gains a _FutureDevelopment helper and parity checks that TriangleWeight-based LDFs match Development across drop scenarios, plus direct TriangleWeight weight tests where _set_weight_func used to be called.

^{Reviewed by Cursor Bugbot for commit 344f22d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 95.29412% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (68b6183) to head (344f22d).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
chainladder/utils/triangle_weight.py	94.89%	5 Missing and 2 partials ⚠️
chainladder/development/tests/test_development.py	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #941 +/- ##
==========================================
+ Coverage 88.48% 89.82% +1.33% 
==========================================
Files 88 90 +2 Lines 5029 7203 +2174 Branches 642 1075 +433 ==========================================
+ Hits 4450 6470 +2020 - Misses 433 523 +90 - Partials 146 210 +64 

Flag	Coverage Δ
unittests	89.80% <95.29%> (+1.32%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kennethshsu]

I really like the functionality here, this makes a lot of sense, but is there really no way reuse the code form the development class? There's so much duplicated code.

I'll open a PR against this (not main)

[henrydingliu]

but is there really no way reuse the code form the development class?

it is

There's so much duplicated code.

simplifying DevelopmentBase is step 5.

• step1: this PR, creating the new TriangleWeight class
  • success criterion: parity across a mass swath of tests
• step2: small refactor of IncrementalAdditive and glm to use new TriangleWeight class
  • success criterion: massive drop in codecov (recycled code from DevelopmentBase no longer used)
• step3: removing recycled code from DevelopmentBase
  • success criterion: restoring % in codecov
• step4: small refactor in Development base to use new TriangleWeight class
  • contingent on: completion of friedland recreation
  • success criterion: no test broken, massive drop in codecov
• step5: removing original weight code from DevelopmentBase
  • success criterion: restoring % in codecov

[kennethshsu]

Ok I think I get what you are doing here, but I don't think introducing a second set of weight assigning functions that are exactly the same as Development Base is the right solution here? I think my PR into this PR is basically keeping step 1, 2, and 3 together. While we don't introduce another set of weight assigning functions to keep track.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

[henrydingliu]

I think my PR into this PR is basically keeping step 1, 2, and 3 together.

but it doesn't. it just keeps the duplicate code within developmentbase.

While we don't introduce another set of weight assigning functions to keep track.

the second set of weight assigning functions were introduced done year ago. this PR finally pulls that second set of weight functions out, in preparation for getting rid of it. please please read the additional context.

A better question is why introduce another set of identical functions if we have to eventually refactor and move out of Development Base?

because a PR with that much change doesn't get reviewed :/ #914 was hanging out there for weeks.

[kennethshsu]

Okayyyy I think I know what you are saying and doing now. I'm not sure if breaking up PRs like this is actually helpful or not. Yes, smaller PRs are easier to review, but I think a consolidated PR is actually best if it does the thing on the ticket.

I took a look at #914 again and I actually think it's a great PR, I think you just caught both @genedan and I at a bad time. And we could probably get more codeowners to review too.

This PR takes steps towards it, but does not arrive it, and I got hung up on trying to understand the duplicated code. Do you have the rest of the parts? Are you doing this on multiple branches? Or do you do this based on commits on the same branch? I think what you can try is submit a PR, and use commits to lay out the steps? I'm not sure what the best practice is here.

[henrydingliu]

i'll combine steps 1 through 3 into this PR. gimme a couple

[cursor]

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 e705d39. Configure here.}

[henrydingliu]

documenting the promised "mass drop in coverage" in step2 before removing those lines in step 3

[henrydingliu]

@kennethshsu this is ready