Skip to content

Pandas annotations#1035

Open
genedan wants to merge 10 commits into
mainfrom
pandas_annotations
Open

Pandas annotations#1035
genedan wants to merge 10 commits into
mainfrom
pandas_annotations

Conversation

@genedan

@genedan genedan commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Summary of Changes

Add type annotations to pandas.py. Add API reference examples to TrianglePandas.dropna(). This method really needed examples to illustrate its complex behavior. I'll try to stick to just 1 or 2 methods next time if they turn out to be as complex.

Related GitHub Issue(s)

#486
#704

Additional Context for Reviewers

Removed line 303:

self.values = np.where((xp.nan_to_num(self.values) == 0) * (self.nan_triangle == 1), self.nan_triangle * 0, self.values)

This line didn't do anything, it simplifies to self.values = self.values, so I removed it.

  • 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
fillna behavior changes (mandatory value and altered inplace math) can affect callers that passed None or relied on the old frame-based fill path.

Overview
Adds static typing across chainladder/core/pandas.py by typing mixin methods with TriangleProtocol, return/cast hints for sparse COO paths, and expanded docstrings for plot, hvplot, and _get_axis.

TriangleProtocol in typing.py is expanded with properties and methods the pandas mixin relies on (origin, development, fillna, arithmetic, indexing, etc.) so type checkers can validate TrianglePandas without Self on older Python.

Triangle.dropna() gains extensive doctest examples documenting edge-trimming behavior; implementation adds clarifying comments and explicit cast on sliced returns.

fillna now requires a fill value (TypeError if None) and uses a simpler inplace update (self + fill instead of an intermediate self + value * 0 frame). fillzero / dropna return paths use cast for checker clarity; to_frame long-format valuation assembly is refactored without intended output changes.

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

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.34884% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.19%. Comparing base (468f11a) to head (1fa4d77).

Files with missing lines Patch % Lines
chainladder/core/pandas.py 95.34% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1035      +/-   ##
==========================================
- Coverage   89.21%   89.19%   -0.03%     
==========================================
  Files          89       89              
  Lines        5165     5173       +8     
  Branches      661      662       +1     
==========================================
+ Hits         4608     4614       +6     
- Misses        390      391       +1     
- Partials      167      168       +1     
Flag Coverage Δ
unittests 89.19% <95.34%> (-0.03%) ⬇️

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.

@genedan

genedan commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator Author

I'll fix that codecov comment in another PR, I think this one's already kind of big.

Examples
--------

In a single-dimension case, an origin period will be dropped if it contains all NaN.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does this happen in the interior of the triangle?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

read the later example. we should clarify here that dropna doesn't work on the interior


If both the earliest origin and development periods are all NaN, both will be dropped.

.. testcode::

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dropna isn't called in this example

columns=['paid'],
cumulative=True
)
print(tri.loc['abc'])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dropna isn't used.

1986 500.0 600.0
1987 500.0 NaN

In the case of a multi-dimensional Triangle, periods will only be dropped if their aggregate sum across

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does this mean if i have a ground-up triangle and a deductible triangle in the same triangle, i could end up with origins or developments dropped?

self.development <= ddim.max())
]
return obj[(self.origin >= min_odim) & (self.origin <= max_odim)]
obj = cast("TriangleProtocol", cast(object, obj))

@henrydingliu henrydingliu Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

two casts (i guess technically 3) in 2 lines. why the differences?

  • one upcasts to object but ones doesn't
  • one casts to triangleprotocal one casts to triangle

return obj

def head(self, n=5):
def head(self: TriangleProtocol, n=5):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is it too trivial to add n:int?

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.

2 participants