Skip to content

[FEAT] Add a duplicates option to the Triangle constructor - #1580

Open
scks1 wants to merge 3 commits into
casact:mainfrom
scks1:fix-clrd-names-1567
Open

scks1 wants to merge 3 commits into
casact:mainfrom
scks1:fix-clrd-names-1567

Conversation

@scks1

@scks1 scks1 commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

Triangle gets a duplicates keyword for input rows that fall in the same index, origin and development cell:

  • "sum" (default) adds them together, as before. With the first commit alone, all 54 bundled samples load bit-identical to main.
  • "raise" raises a ValueError naming up to three repeated cells and their row counts.
  • "warn" gives the same message as a warning, then sums.

cl.read_csv takes the keyword too, and cl.load_sample passes it on. The Triangle docstring and the user guide's triangle page document it.

The second commit is the GRNAME rename discussed in #1567, in clrd.csv and clrd2025.csv: 10323 → Eagle Mut, 32670 → Agraria Ins Co, 17124 → Farmers Mut Ins Co (defunct), 14443 → Madison Mut Ins Co (IL) and 30449 → Madison Mut Ins Co (NY). No other field changes. clrd goes from 775 to 779 triangles and clrd2025 from 768 to 772, and both load with duplicates="raise". Before the rename, clrd has 220 repeated cells. The docstring examples and stored notebook counts are updated. Drop that commit to leave the samples as they are.

Related GitHub Issue(s)

Closes #1567.

AI/LLM Usage

This change was made with Árbol, an AI assistant running on Muse, working under my direction. It wrote the change and the tests, ran the checks below and drafted this description.

Additional Context for Reviewers

  1. Rows are counted with the keys _aggregate_data sums by, after dates move to their period's start or end, so two payment dates in one month of a monthly triangle count as repeated. The extra groupby(...).size() runs only when duplicates is not "sum".
  2. _split_ult passes the keyword on, so ultimate rows are checked too. DevelopmentML and read_json keep "sum", since they rebuild triangles from the package's own one-row-per-cell output.
  3. Two triangle-tutorial cells that sum the PaidToInc ratio column change in the seventh digit after the rename. They were left for the docs build, which re-executes the notebooks.

test_triangle.py and test_utilities.py pass with numpy 2.5.4 and pandas 3.1.0 and with the uv.lock versions (numpy 2.3.3, pandas 2.3.3), and with the rename so do the tests that load clrd. Doctest builds of the changed docstrings pass, and ruff is clean.

Submitter's Checklist

  • I have reviewed and am adhering to the standards outlined in the project Governing Doc.
  • The PR subject title summarizes the changes, with one proper prefix ([FIX], [FEAT], [DOCS], [TST], [CHORE], or [BRK]).
  • I am a human (not a bot), and this PR form is written by a human.

Reviewer's Checklist

  • The implementation addresses the associated issue(s).
  • The implementation is appropriate, maintainable, and follows ARCHITECTURE.md.
  • PR subject title has the proper prefix and the subject is appropriate.
  • Relevant issue(s) are linked.
  • AI/LLM usage is disclosed and appropriate.
  • Documentation and tests are appropriate.
  • CI tests passed, or any failures are acceptable.
  • Leave a comment with the final recommendation (e.g. approve as is, request a secondary review, or flag an area for more review).

Triangle adds together every input row that falls in the same index,
origin and development cell. Transaction data needs that, but in data
meant to hold one row per cell a repeated key is an error that the sum
hides.

duplicates="sum" (the default) keeps that behavior. duplicates="raise"
raises a ValueError naming up to three repeated cells and their row
counts, and duplicates="warn" gives the same message as a warning and
then sums. The rows are counted where _aggregate_data groups them, at
the triangle's grain, on both its branches. The keyword passes through
_split_ult and cl.read_csv; cl.load_sample forwards it already. The
user guide's triangle page gets a short paragraph on it.

Refs casact#1567.

Developed with an AI assistant (Árbol, running Muse).
clrd.csv and clrd2025.csv each give three company names to two group
codes: Farmers Mut Ins Co (10323, 17124), Farmers Union Mut Ins Co
(28436, 32670) and Madison Mut Ins Co (14443, 30449). With the samples'
index ["GRNAME", "LOB"], load_sample summed the rows of the two
companies in four name and line pairs.

Five codes get new names: 10323 Eagle Mut and 32670 Agraria Ins Co, as
in the NAIC listing; 17124, which the listing does not have, Farmers
Mut Ins Co (defunct); and 14443 and 30449 Madison Mut Ins Co (IL) and
Madison Mut Ins Co (NY). Only the GRNAME field of those codes' rows
changes: 385 rows in clrd.csv and 690 in clrd2025.csv. clrd now has 779
triangles instead of 775 and clrd2025 772 instead of 768, and both
load with duplicates="raise".

The shape doctest in base.py, ClarkLDF's prose and output, and the
standalone column of Development's example are updated, as are the
stored triangle counts in six notebooks: the triangle and development
tutorials, the user guide's triangle, development and workflow pages,
and the triangle-from-pandas gallery example. The triangle tutorial's
prose count changes with them. test_duplicates_multi_index builds its
repeated rows itself now that clrd.csv has none, and
test_clrd_samples_one_row_per_cell checks the new counts.

Refs casact#1567.

Developed with an AI assistant (Árbol, running Muse).
@github-actions

github-actions Bot commented Nov 11, 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): 17.3% of exported symbols fully typed (276 / 1599)

Known Ambiguous Unknown Total
Project (head) 276 101 1222 1599

Other symbols referenced but not exported by chainladder: 24

Known Ambiguous Unknown Total
Other (head) 11 1 12 24

Symbols without documentation:

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

Patch (exported symbols added or changed by this PR): 84.6% fully typed (11 / 13)

Known Ambiguous Unknown Total
Patch 11 0 2 13
Patch symbol details
Symbol Status Change
chainladder.core.tests.test_triangle.DUPLICATE_KWARGS ❌ unknown new
chainladder.core.tests.test_triangle.DUPLICATE_ROWS ✅ known new
chainladder.core.tests.test_triangle.SAMPLE_DATA ✅ known new
chainladder.core.tests.test_triangle.test_clrd_samples_one_row_per_cell ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_claim_level ❌ unknown new
chainladder.core.tests.test_triangle.test_duplicates_invalid_value ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_multi_index ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_pandas_subclass ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_raise_names_cells ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_sum_by_default ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_ultimate_rows ✅ known new
chainladder.core.tests.test_triangle.test_duplicates_warn_sums ✅ known new
chainladder.core.tests.test_triangle.test_read_csv_duplicates ✅ known new

@codecov

codecov Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.78%. Comparing base (8e2e53c) to head (12dd4e1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1580      +/-   ##
==========================================
+ Coverage   94.59%   94.78%   +0.18%     
==========================================
  Files          98       98              
  Lines        6233     6344     +111     
  Branches      799      810      +11     
==========================================
+ Hits         5896     6013     +117     
+ Misses        216      212       -4     
+ Partials      121      119       -2     
Flag Coverage Δ
unittests 94.78% <100.00%> (+0.18%) ⬆️

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.

Comment thread chainladder/core/triangle.py Outdated
``cl.options.AUTO_SPARSE`` is ``True``, the backend is instead
chosen automatically based on array size and density, regardless
of this argument's value.
duplicates: str, default 'sum'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a Literal would be better

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 12dd4e1. duplicates is now typed Literal["sum", "raise", "warn"] in Triangle.__init__, read_csv and the three internal helpers that pass it on, and both docstrings give the type as {'sum', 'raise', 'warn'}, as Triangle.drop does for errors.

(This reply was made with Árbol, an AI assistant, under my direction.)

Comment thread chainladder/utils/utility_functions.py Outdated
pattern=False,
trailing: bool = True,
age: Optional[str] = None,
duplicates: str = "sum",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same as previous comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed here too, in the same commit.

(This reply was made with Árbol, an AI assistant, under my direction.)

Comment thread chainladder/core/tests/test_triangle.py Outdated

def test_duplicates_transactions(monthly: Triangle) -> None:
"""
Transaction data put many rows in each cell. Built from prism.csv with no

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

small gripe. prism isn't what would conventionally be considered transactional data

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair point: prism.csv has one row per claim. In 12dd4e1 the test is renamed test_duplicates_claim_level and its docstring says the rows are claims, and the user guide paragraph that called prism transaction data now calls it claim-level data.

(This reply was made with Árbol, an AI assistant, under my direction.)

- duplicates is annotated Literal["sum", "raise", "warn"] in
  Triangle.__init__, read_csv and the helpers that pass it on, and the
  docstrings give its type as {'sum', 'raise', 'warn'}.
- prism.csv holds one row per claim, so the test and the user guide call
  it claim-level data; the test is renamed test_duplicates_claim_level.

Developed with an AI assistant (Árbol, running Muse).
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.

[FEAT] Opt-in check for duplicate keys in the Triangle constructor

2 participants