Skip to content

[Fix]Triangle abstract-class diagnostics in static type checkers - #1525

Open
yuuuxt wants to merge 3 commits into
casact:mainfrom
yuuuxt:fix-abstract-class
Open

yuuuxt wants to merge 3 commits into
casact:mainfrom
yuuuxt:fix-abstract-class

Conversation

@yuuuxt

@yuuuxt yuuuxt commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

Related GitHub Issue(s)

Closes #1524

AI/LLM Usage

The fix is proposed by LLM and reviewed manually. Confirmed the issue is resolved. Please review further for correctness.

Additional Context for Reviewers

(Below from LLM explanation)

This patch fixes the type-checking view of TrianglePandas:

  • Put Common before the protocol base in TrianglePandas’s inheritance order, so type checkers resolve the concrete set_backend implementation before the protocol declaration.
  • Declare sum under TYPE_CHECKING, since it is added dynamically at runtime and otherwise isn’t recognized by static analysis.

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 2, 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): 16.1% of exported symbols fully typed (247 / 1534)

Known Ambiguous Unknown Total
Project (head) 247 106 1181 1534

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

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

Known Ambiguous Unknown Total
Patch 0 0 2 2
Patch symbol details
Symbol Status Change
chainladder.core.pandas.TrianglePandas.set_backend ❌ unknown new
chainladder.core.pandas.TrianglePandas.sum ❌ unknown new

@codecov

codecov Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.36%. Comparing base (2ea53be) to head (4de959e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1525   +/-   ##
=======================================
  Coverage   94.36%   94.36%           
=======================================
  Files          97       97           
  Lines        6198     6198           
  Branches      798      798           
=======================================
  Hits         5849     5849           
  Misses        225      225           
  Partials      124      124           
Flag Coverage Δ
unittests 94.36% <ø> (ø)

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

@yuuuxt thanks for the deep finds! a couple questions

  • do we achieve the same outcome if we declare set_backend (similar to declaring sum) under TYPE_CHECKING, instead of adding the Common typing?
  • do we need other agg functions declared under TYPE_CHECKING in addition to sum?

@yuuuxt

yuuuxt commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

@yuuuxt thanks for the deep finds! a couple questions

* do we achieve the same outcome if we declare `set_backend` (similar to declaring `sum`) under `TYPE_CHECKING`, instead of adding the `Common` typing?

* do we need other agg functions declared under `TYPE_CHECKING` in addition to `sum`?
  • You are right this is a cleaner way, updated.
  • only sum is required because Triangle gets flagged as abstract because two protocol methods have no visible implementation: set_backend and sum (pyright and ty list both). For this issue only adding sum is enough. (LLM investigates saying there are other related issues, I'm reading on its answer)

(Disclaimer: as you see it is heavily assisted by LLM with some of my manual checking)


Other issues found by LLM are created, after manually checking the repros: #1526, #1527, #1528.

@henrydingliu henrydingliu 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.

appears inoffensive. @genedan does this interfere/conflict with your existing typing plans?

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.

Triangle is reported as abstract by Pyright/Pylance and ty

2 participants