Conversation
Pyright Type CompletenessView the full Project (full
Other symbols referenced but not exported by
Symbols without documentation:
Patch (exported symbols added or changed by this PR): 38.5% fully typed (10 / 26)
Patch symbol details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1342 +/- ##
==========================================
+ Coverage 91.76% 91.83% +0.07%
==========================================
Files 96 98 +2
Lines 5475 5525 +50
Branches 706 712 +6
==========================================
+ Hits 5024 5074 +50
Misses 327 327
Partials 124 124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 53f1edc. Configure here.
| self, | ||
| key: str | int, | ||
| value: int | float | TriangleSlicer | Callable[[Triangle], TriangleSlicer], | ||
| ) -> None: ... |
There was a problem hiding this comment.
can you confirm this is ruff formatting changes only?
|
|
||
| def __init__( | ||
| self, | ||
| data: pd.DataFrame | pd.Series, |
There was a problem hiding this comment.
I think the triangle argument is the extraneous one. I can eliminate it, and restrict data so that it still matches the Pandas API, but only takes a Triangle since a regular DataFrame or Series doesn't have components like a lower diagonal.
| styler = styler.format( # pyright: ignore[reportReturnType] | ||
| fmt_str, na_rep="" | ||
| ) | ||
| return styler |
There was a problem hiding this comment.
is there any value in allowing styler on multi triangle but essentially blocking multi triangle from using any of the custom cl stylers? (i know we only have a single styler right now. but anything we might add, like highlight_diagonal, etc. would all only work on single triangles) like, what if we just let multi triangles error out right here
There was a problem hiding this comment.
I've updated the code to throw an exception if the triangle is multi. Although I do need your thought on e putting "2-D" in the error message. It's technically 2-D (origin x development), but it's often called 1-D throughout the code. I think "single triangle" is a little less controversial.
There was a problem hiding this comment.
'single'/'multi' is how we are calling it in the codebase. i'm good with single.
| from chainladder.core.style import Styler | ||
|
|
||
| df = pd.DataFrame({"a": [1, 2], "b": [3, 4]}) | ||
| Styler(df).format(precision=1) |
There was a problem hiding this comment.
can this mimic the pandas doc and actually show an output? something like precision = 1 on a raa.age-to-age
There was a problem hiding this comment.
Yeah it can, I'll need to get rid of this example though. We really shouldn't be allowing the Styler to take a regular DataFrame or Series since it's designed to work on the Triangle-specific attributes. I'll replace this with a precision example (will add the feature if needed).
| full.style.highlight_lower_triangle( | ||
| color="lightgray", valuation_date=raa.valuation_date | ||
| ) | ||
|
|
There was a problem hiding this comment.
can we follow pandas and link to the tutorial notebook?
| if text_color is not None: | ||
| props += f" color: {text_color};" | ||
|
|
||
| def f(_data: pd.DataFrame) -> np.ndarray: |
There was a problem hiding this comment.
i feel like we are gatekeeping a lot of generalizable code inside this single-purpose styler. how about overloading or replacing cl.Styler.apply with apply(indicator_triangle, true_prop, false_prop)? then highlight_lower_triangle can route through that apply.
There was a problem hiding this comment.
We can get rid of this f function and create a generalized method to create an indicator triangle, then pass it to apply. Will let you know when it's done.
2 participants
Summary of Changes
Adds functionality to style Triangles directly.
Related GitHub Issue(s)
Closes:
#1332
#1333
#1324
Partially addresses:
#1337
Additional Context for Reviewers
The docs have a stub tutorial in
user_guide/style.ipynb. This allows the reviewer to visually inspect what the new styling looks like. I'll fill it out with a proper tutorial towards the end of the parent issue #1331.I recommend the reviewer to first take a look at the notebook to confirm the feature works.
pandas.pyis whereTriangle.style()returns the styler, which is defined instyle.py(where the bulk of the formatting logic takes place).Checklist
uv run pytest) and documentation changes (uv run --directory docs jb build . --builder=custom --custom-builder=doctest)Note
Low Risk
Additive presentation API with no changes to reserving math; lower-triangle masking is guarded by shape checks and clear errors when misused.
Overview
Adds a pandas-style API for styling triangles: a public
Stylersubclass ofpandas.io.formats.style.Stylerand aTriangle.styleproperty (mirroringDataFrame.style) that wrapsto_frame()and applies default number formatting for single-dimension triangles.The Triangle-specific
highlight_lower_trianglemethod CSS-highlights cells beyond the latest diagonal using the wrapped triangle’s valuation grid (defaultvaluation_date), with optionaltext_color, customprops, and an explicitvaluation_datefor fully predicted triangles likefull_triangle_. It requires a styler fromTriangle.styleand only supports a single 2-D grid.Styleris exported fromchainladder, documented in the API and a stubuser_guide/style.ipynb, with broad unit tests covering subclass behavior, masking alignment withnan_triangle, and error cases for non-triangle stylers and multi-dimensional frames.Reviewed by Cursor Bugbot for commit 97b3771. Bugbot is set up for automated code reviews on this repo. Configure here.