Skip to content

Feature/deprecate date dims - #1304

Draft
priyam0k wants to merge 4 commits into
feature/deprecate-dimsfrom
feature/deprecate-date-dims
Draft

priyam0k wants to merge 4 commits into
feature/deprecate-dimsfrom
feature/deprecate-date-dims

Conversation

@priyam0k

@priyam0k priyam0k commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Deprecate odims (Deprecate odims #1012) and ddims (Deprecate ddims #1013) on Triangle with FutureWarnings pointing to origin and development
  • Update internal usage across core modules, estimators, adjustments, and tails to prevent internal warnings
  • Preserve legacy pickle compatibility in Triangle.setstate so older saved instances unpickle cleanly
  • Add test coverage verifying deprecation warnings when reading or setting both attributes and unpickling legacy objects
  • Address ruff lint and formatting on touched files

Related GitHub Issue(s)

Closes #1012, closes #1013, refs #601, refs #1216

Additional Context for Reviewers

Checklist

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

Note

Medium Risk
Wide changes to how origin/development axes are stored and mutated across reserving and triangle arithmetic, though behavior is intended to stay backward compatible via deprecated accessors and pickle migration.

Overview
Deprecates public Triangle.odims and Triangle.ddims, moving internal axis storage to private _odims and _ddims while steering callers toward origin and development.

Read/write access to the old names still works but emits FutureWarning. Library code across core, development, methods, tails, adjustments, and utilities is updated to use the private fields so internals no longer trigger deprecation noise. Slicing helpers accept both legacy and private axis names for compatibility.

Pickle migration in Triangle.__setstate__ maps serialized odims/ddims into _odims/_ddims, with expanded tests for warnings, legacy unpickling, and JSON checks using origin/development. A few unrelated test fixes (R import guard, TailCurve fit period) ride along.

Reviewed by Cursor Bugbot for commit a2ce5cd. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.63057% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.59%. Comparing base (f711b88) to head (a2ce5cd).

Files with missing lines Patch % Lines
chainladder/development/munich.py 55.55% 4 Missing ⚠️
chainladder/adjustments/bootstrap.py 0.00% 0 Missing and 1 partial ⚠️
chainladder/development/incremental.py 50.00% 0 Missing and 1 partial ⚠️
chainladder/methods/base.py 66.66% 0 Missing and 1 partial ⚠️
chainladder/tails/constant.py 50.00% 1 Missing ⚠️
chainladder/tails/curve.py 50.00% 1 Missing ⚠️
chainladder/tails/tests/rtest_exponential.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           feature/deprecate-dims    #1304      +/-   ##
==========================================================
+ Coverage                   92.57%   92.59%   +0.02%     
==========================================================
  Files                         100      100              
  Lines                        6020     6040      +20     
  Branches                      796      798       +2     
==========================================================
+ Hits                         5573     5593      +20     
  Misses                        326      326              
  Partials                      121      121              
Flag Coverage Δ
unittests 92.59% <93.63%> (+0.02%) ⬆️

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.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Pyright Type Completeness

View the full pyright --verifytypes output for this commit

Project (full chainladder package, at this PR's head): 14.7% of exported symbols fully typed (207 / 1408)

Known Ambiguous Unknown Total
Project (head) 207 110 1091 1408

Other symbols referenced but not exported by chainladder: 13

Known Ambiguous Unknown Total
Other (head) 3 1 9 13

Symbols without documentation:

  • Functions without docstring: 328
  • Functions without default param: 0
  • Classes without docstring: 10

Patch (exported symbols added or changed by this PR): 0.0% fully typed (0 / 4)

Known Ambiguous Unknown Total
Patch 0 0 4 4
Patch symbol details
Symbol Status Change
chainladder.core.tests.test_triangle.test_ddims_deprecation_warning ❌ unknown new
chainladder.core.tests.test_triangle.test_odims_deprecation_warning ❌ unknown new
chainladder.core.triangle.Triangle.ddims ❌ unknown changed (was ✅ known)
chainladder.core.triangle.Triangle.odims ❌ unknown changed (was ✅ known)

@priyam0k
priyam0k changed the base branch from main to feature/deprecate-dims September 5, 2026 16:55
@priyam0k
priyam0k force-pushed the feature/deprecate-date-dims branch from 7bcf3d0 to 0c87d2e Compare September 5, 2026 20:10

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread chainladder/development/constant.py Outdated
@priyam0k
priyam0k force-pushed the feature/deprecate-date-dims branch from a9f5c57 to c6e0c8d Compare September 8, 2026 23:31
@priyam0k
priyam0k force-pushed the feature/deprecate-date-dims branch 4 times, most recently from 7d7efdc to 4253b6e Compare September 9, 2026 12:37
…1012, #1013)

- Encapsulate internal date dimensions into _odims and _ddims
- Provide backward-compatible odims and ddims properties emitting FutureWarning
- Migrate legacy pickled instances with odims/ddims in Triangle.__setstate__
- Decouple internal usages across core, estimators, tails, and tests
- Address ruff linting and formatting on touched files

Closes #1012, closes #1013, refs #1216, refs #601

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a2ce5cd. Configure here.

if not obj.is_full:
obj = obj[obj.valuation < obj.valuation_date]
if hasattr(obj, "w_"):
w_ = obj.w_[..., : len(obj.odims), :]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest_diagonal triggers deprecation warning

Medium Severity

latest_diagonal still assigns through the deprecated ddims setter, so a FutureWarning fires on every access even though callers never use odims or ddims. That warning then leaks into other core methods that rely on latest_diagonal.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a2ce5cd. Configure here.

@priyam0k
priyam0k marked this pull request as draft September 20, 2026 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate ddims Deprecate odims

2 participants