Skip to content

Add type annotations to sympy.logic.inference - #30532

Open
MateoLauzardo wants to merge 1 commit into
sympy:masterfrom
MateoLauzardo:typing-28806
Open

MateoLauzardo wants to merge 1 commit into
sympy:masterfrom
MateoLauzardo:typing-28806

Conversation

@MateoLauzardo

@MateoLauzardo MateoLauzardo commented Sep 18, 2026

Copy link
Copy Markdown

References to other Issues or PRs

See #28806

Brief description of what is fixed or changed

Adds type annotations to sympy/logic/inference.py and release/authors.py.
In inference.py I annotated valid, pl_true, its inner _validate, and
entails, plus a Model alias for the assignment dicts pl_true passes
around; in authors.py, blue, red, and get_authors. I left
literal_symbol and satisfiable alone — satisfiable's return type varies
with all_models and the chosen algorithm, so it needs overloads rather than
one signature, which felt like its own change.

Other comments

pl_true rebound expr and model to different types inside the body, which
mypy rejects. Rather than widen the parameter types, I gave the intermediates
new names — sexpr for the sympified expression, deep_model for the atoms
dict — so the parameters keep the types in their signature. No behaviour
change, but it's the one spot where the diff is more than added annotations.

While typing the return I noticed pl_true isn't consistent about boolean
results: an input already in (True, False) comes back as-is via return expr, while a result that reduces after subs goes through bool(result).
Because BooleanTrue compares equal to True, pl_true(true) returns
BooleanTrue but pl_true(x, {x: True}) returns a Python bool. I typed the
return as Boolean | bool | None to cover both. Would you rather narrow it to
bool | None and convert in the first branch, or is the current behaviour
depended on somewhere?

AI Generation Disclosure

Claude Code (Claude Opus 5), run in VSCode against a local clone.

sympy/logic/inference.py — AI-generated: the annotations on valid, entails,
pl_true and _validate; the Model alias and TYPE_CHECKING block; and the
pl_true body changes (sexpr, deep_model, and the cast on subs).

release/authors.py — AI-generated: the annotations on blue, red and
get_authors, and the added future import.

I reviewed every line. mypy sympy reports no errors across 1553 files,
ruff check sympy passes, sympy/logic/tests gives 136 passed, and
bin/doctest sympy/logic/inference.py and bin/test quality both pass.

Release Notes

NO ENTRY

Annotate valid, entails and pl_true. pl_true's body was rebinding its parameters to wider types, so intermediate values now use separate local names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 18, 2026 02:34

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sympy-bot

sympy-bot commented Sep 18, 2026

Copy link
Copy Markdown

Hi, I am the SymPy bot. I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

  • No release notes entry will be added for this pull request.
Click here to see the pull request description that was parsed.
#### References to other Issues or PRs

See #28806

#### Brief description of what is fixed or changed

Adds type annotations to `sympy/logic/inference.py` and `release/authors.py`.
In `inference.py` I annotated `valid`, `pl_true`, its inner `_validate`, and
`entails`, plus a `Model` alias for the assignment dicts `pl_true` passes
around; in `authors.py`, `blue`, `red`, and `get_authors`. I left
`literal_symbol` and `satisfiable` alone — `satisfiable`'s return type varies
with `all_models` and the chosen algorithm, so it needs overloads rather than
one signature, which felt like its own change.

#### Other comments

`pl_true` rebound `expr` and `model` to different types inside the body, which
mypy rejects. Rather than widen the parameter types, I gave the intermediates
new names — `sexpr` for the sympified expression, `deep_model` for the atoms
dict — so the parameters keep the types in their signature. No behaviour
change, but it's the one spot where the diff is more than added annotations.

While typing the return I noticed `pl_true` isn't consistent about boolean
results: an input already in `(True, False)` comes back as-is via `return
expr`, while a result that reduces after `subs` goes through `bool(result)`.
Because `BooleanTrue` compares equal to `True`, `pl_true(true)` returns
`BooleanTrue` but `pl_true(x, {x: True})` returns a Python `bool`. I typed the
return as `Boolean | bool | None` to cover both. Would you rather narrow it to
`bool | None` and convert in the first branch, or is the current behaviour
depended on somewhere?

#### AI Generation Disclosure

Claude Code (Claude Opus 5), run in VSCode against a local clone.

sympy/logic/inference.py — AI-generated: the annotations on valid, entails,
pl_true and _validate; the Model alias and TYPE_CHECKING block; and the
pl_true body changes (sexpr, deep_model, and the cast on subs).

release/authors.py — AI-generated: the annotations on blue, red and
get_authors, and the added __future__ import.

I reviewed every line. mypy sympy reports no errors across 1553 files,
ruff check sympy passes, sympy/logic/tests gives 136 passed, and
bin/doctest sympy/logic/inference.py and bin/test quality both pass.


#### Release Notes

<!-- BEGIN RELEASE NOTES -->

NO ENTRY

<!-- END RELEASE NOTES -->

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.

3 participants