Skip to content

Add index, columns, origin, development alternatives to Triangle.drop()#1131

Open
priyam0k wants to merge 4 commits into
casact:mainfrom
priyam0k:feature/drop-axis-alternatives
Open

Add index, columns, origin, development alternatives to Triangle.drop()#1131
priyam0k wants to merge 4 commits into
casact:mainfrom
priyam0k:feature/drop-axis-alternatives

Conversation

@priyam0k

@priyam0k priyam0k commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

Adds index, columns, origin, and development keyword alternatives to Triangle.drop(), mirroring DataFrame.drop().

Related GitHub Issue(s)

closes #1060, part of #1052

Additional Context for Reviewers


Note

Low Risk
Column-drop behavior for the legacy API is preserved; new kwargs mainly add routing and errors on non-column axes, with coverage in unit tests.

Overview
Triangle.drop() now accepts index, columns, origin, and development as pandas-style shortcuts (e.g. tri.drop(columns='CumPaidLoss') instead of labels=..., axis=1). Mixing labels with any of those kwargs raises ValueError.

Implementation builds a per-axis drop map and normalizes scalars with np.isscalar; only the column axis still drops data—other axes route through the same path and raise NotImplementedError until follow-up work lands. The existing labels/axis API is unchanged.

TriangleProtocol and doctest examples were updated; tests cover equivalence, list/index-like columns, conflicts, and unimplemented axes.

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

…ngle.drop()

Route the keyword alternatives to their axis, mirroring pandas. Refs casact#1060.
@github-actions

github-actions Bot commented Jul 15, 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.8% of exported symbols fully typed (191 / 1287)

Known Ambiguous Unknown Total
Project (head) 191 110 986 1287

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: 312
  • Functions without default param: 0
  • Classes without docstring: 10

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

Known Ambiguous Unknown Total
Patch 0 0 6 6
Patch symbol details
Symbol Status Change
chainladder.core.tests.test_triangle.test_drop_columns_alternative ❌ unknown new
chainladder.core.tests.test_triangle.test_drop_columns_alternative_index_like ❌ unknown new
chainladder.core.tests.test_triangle.test_drop_columns_alternative_list ❌ unknown new
chainladder.core.tests.test_triangle.test_drop_index_origin_development_alternatives_raise ❌ unknown new
chainladder.core.tests.test_triangle.test_drop_integer_label_routes_to_axis ❌ unknown new
chainladder.core.tests.test_triangle.test_drop_labels_and_alternative_raises ❌ unknown new

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.82%. Comparing base (4533ef6) to head (8157830).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1131      +/-   ##
==========================================
+ Coverage   90.81%   90.82%   +0.01%     
==========================================
  Files          91       91              
  Lines        5300     5308       +8     
  Branches      673      676       +3     
==========================================
+ Hits         4813     4821       +8     
  Misses        346      346              
  Partials      141      141              
Flag Coverage Δ
unittests 90.82% <100.00%> (+0.01%) ⬆️

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.

@henrydingliu

Copy link
Copy Markdown
Member

@genedan wanna take this one?

@genedan

genedan commented Jul 16, 2026

Copy link
Copy Markdown
Member

Okay, I'm gonna help knock out the contributing guide draft beforehand. Might take me a day or two? I figured I need to get on it since we keep talking about it.

@genedan genedan left a comment

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.

Finally got around to this, many thanks for the PR. I only have minor comments.

Comment thread chainladder/core/typing.py Outdated
Comment thread chainladder/core/pandas.py
Address review on casact#1131: format the long drop() signature one arg per line and add testable Examples to the docstring.
Comment thread chainladder/core/pandas.py
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 casact#1131.

@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 2a7c50e. Configure here.

Comment thread chainladder/core/pandas.py
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 casact#1131.
@priyam0k
priyam0k requested a review from genedan July 22, 2026 18:29
.. testcode::

tri = cl.load_sample('clrd')
print(tri.drop(columns='CumPaidLoss').columns.tolist())

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.

I would recommend adding 1 more print statement showing the original columns of the triangle.

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.

Add index, columns, origin, development alternatives to Triangle.drop()

3 participants