Skip to content

[FIX] Runtime-attached Triangle methods are unknown to Pyright/Pylance and ty - #1559

Open
yuuuxt wants to merge 6 commits into
casact:mainfrom
yuuuxt:issue-1526
Open

yuuuxt wants to merge 6 commits into
casact:mainfrom
yuuuxt:issue-1526

Conversation

@yuuuxt

@yuuuxt yuuuxt commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary of Changes

Related GitHub Issue(s)

Closes #1526

AI/LLM Usage

Created by LLM and manually reviewed

Additional Context for Reviewers

  • The sum of TrianglePandas was returning TriangleProtocol, now it is returning Triangle which is more appropriate (repro: check the triangle.sum().mean() line in issue).
  • The inner function of add_triangle_agg_func was typed returning Triangle | ndarray,and now it is extracted to be _triangle_aggregate returning Triangle. LLM notes the old typing was wrong as well, so I left the type change here.
  • The added tests are not carefully checked. Tests (being) reviewed by @henrydingliu

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).

@github-actions

github-actions Bot commented Nov 9, 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.6% of exported symbols fully typed (289 / 1644)

Known Ambiguous Unknown Total
Project (head) 289 96 1259 1644

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

Patch (exported symbols added or changed by this PR): 34.1% fully typed (15 / 44); 9 no longer exported

Known Ambiguous Unknown Total
Patch 15 0 29 44
Patch symbol details
Symbol Status Change
chainladder.core.pandas.TriangleGroupBy.cumsum ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.max ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.mean ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.median ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.min ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.prod ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.quantile ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.std ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.sum ❌ unknown new
chainladder.core.pandas.TriangleGroupBy.var ❌ unknown new
chainladder.core.pandas.TrianglePandas.cumsum ❌ unknown new
chainladder.core.pandas.TrianglePandas.describe ✅ known new
chainladder.core.pandas.TrianglePandas.diff ❌ unknown new
chainladder.core.pandas.TrianglePandas.drop_duplicates ✅ known new
chainladder.core.pandas.TrianglePandas.max ❌ unknown new
chainladder.core.pandas.TrianglePandas.mean ❌ unknown new
chainladder.core.pandas.TrianglePandas.median ❌ unknown new
chainladder.core.pandas.TrianglePandas.melt ✅ known new
chainladder.core.pandas.TrianglePandas.min ❌ unknown new
chainladder.core.pandas.TrianglePandas.pct_change ✅ known new
chainladder.core.pandas.TrianglePandas.pivot ✅ known new
chainladder.core.pandas.TrianglePandas.prod ❌ unknown new
chainladder.core.pandas.TrianglePandas.quantile ❌ unknown new
chainladder.core.pandas.TrianglePandas.std ❌ unknown new
chainladder.core.pandas.TrianglePandas.sum ❌ unknown changed (was ✅ known)
chainladder.core.pandas.TrianglePandas.to_clipboard ✅ known new
chainladder.core.pandas.TrianglePandas.to_csv ✅ known new
chainladder.core.pandas.TrianglePandas.to_dict ❌ unknown new
chainladder.core.pandas.TrianglePandas.to_excel ✅ known new
chainladder.core.pandas.TrianglePandas.to_html ✅ known new
chainladder.core.pandas.TrianglePandas.to_json ✅ known new
chainladder.core.pandas.TrianglePandas.unstack ✅ known new
chainladder.core.pandas.TrianglePandas.var ❌ unknown new
chainladder.core.tests.test_pandas.AGGREGATIONS ✅ known new
chainladder.core.tests.test_pandas.PASSTHRU_METHODS ✅ known new
chainladder.core.tests.test_pandas.PASSTHRU_METHODS_COVERED_BY_TEST_TRIANGLE ✅ known new
chainladder.core.tests.test_pandas.PASSTHRU_METHODS_NOT_COVERED_BY_TEST_TRIANGLE ✅ known new
chainladder.core.tests.test_pandas.test_aggregation_default_axis_first_non_singleton ❌ unknown new
chainladder.core.tests.test_pandas.test_aggregation_default_axis_singleton_result ❌ unknown new
chainladder.core.tests.test_pandas.test_dataframe_passthru_dispatch ❌ unknown new
chainladder.core.tests.test_pandas.test_groupby_aggregation_dispatch ❌ unknown new
chainladder.core.tests.test_pandas.test_pandas_methods_are_defined_on_classes ❌ unknown new
chainladder.core.tests.test_pandas.test_triangle_aggregation_dispatch ❌ unknown new
chainladder.core.tests.test_pandas.test_triangle_aggregation_values ❌ unknown new
chainladder.core.pandas.add_df_passthru — no longer exported (was ❌ unknown)
chainladder.core.pandas.add_groupby_agg_func — no longer exported (was ❌ unknown)
chainladder.core.pandas.add_triangle_agg_func — no longer exported (was ❌ unknown)
chainladder.core.pandas.agg_funcs — no longer exported (was ⚠️ ambiguous)
chainladder.core.pandas.df_passthru — no longer exported (was ⚠️ ambiguous)
chainladder.core.pandas.func — no longer exported (was ⚠️ ambiguous)
chainladder.core.pandas.method — no longer exported (was ⚠️ ambiguous)
chainladder.core.pandas.more_aggs — no longer exported (was ⚠️ ambiguous)
chainladder.core.pandas.set_method — no longer exported (was ❌ unknown)

@yuuuxt yuuuxt changed the title refactor [FIX] Runtime-attached Triangle methods are unknown to Pyright/Pylance and ty Nov 9, 2026
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.65772% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.80%. Comparing base (0e5a7a9) to head (37336f6).

Files with missing lines Patch % Lines
chainladder/core/pandas.py 98.54% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1559      +/-   ##
==========================================
+ Coverage   94.76%   94.80%   +0.04%     
==========================================
  Files          98       99       +1     
  Lines        6318     6376      +58     
  Branches      804      802       -2     
==========================================
+ Hits         5987     6045      +58     
- Misses        212      216       +4     
+ Partials      119      115       -4     
Flag Coverage Δ
unittests 94.80% <98.65%> (+0.04%) ⬆️

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.

@yuuuxt
yuuuxt marked this pull request as ready for review November 9, 2026 16:34
Comment thread chainladder/core/tests/test_pandas.py Resolved
Comment thread chainladder/core/tests/test_pandas.py Resolved
Comment thread chainladder/core/tests/test_pandas.py Resolved
Comment thread chainladder/core/tests/test_pandas.py Resolved
Comment thread chainladder/core/tests/test_pandas.py Resolved
@henrydingliu henrydingliu self-assigned this Nov 10, 2026
@yuuuxt

yuuuxt commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @henrydingliu for reviewing the tests! I made updates accordingly (with LLM), and please continue the review.

@henrydingliu

Copy link
Copy Markdown
Member

i'm good with the code.

two doc-ish changes i'd like to ask for this PR:

  • docstrings for the tests that describe the intent of the test in a bit more detail than just the test names
  • fill in the summary for this PR

things i'd appreciate in future PRs:

  • if your AI agent came up with a bunch of tests that look pretty dense, get your agent to explain and/or get another agent to check it. we generally discourage straight AI-passthrough.
  • your agent workflow is removing a bunch of the resolved comments, which make it harder for me to validate that the resolutions are sufficient.
    • for example, here is one of the comments that did show up in the diff after the update
image image - and here is one of the comments that didn't show up in the diff after the update image image

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.

Runtime-attached Triangle methods (mean, to_csv, groupby aggregations, etc.) are unknown to Pyright/Pylance and ty

2 participants