fix!: reject non-Mapping 'values' on Solver.__call__ with a clear TypeError#16
Draft
hozblok wants to merge 1 commit into
Draft
fix!: reject non-Mapping 'values' on Solver.__call__ with a clear TypeError#16hozblok wants to merge 1 commit into
hozblok wants to merge 1 commit into
Conversation
This was referenced May 11, 2026
…eError
BREAKING CHANGE: Solver.__call__ now requires the 'values' argument to
be a Mapping (or None). Passing a scalar — int, float, str, list — now
raises TypeError with a message that names the parameter and the
offending type. The previous 1-variable shortcut, where a scalar `v`
was silently coerced into `{only_var: str(v)}`, is also removed: the
API is now consistently dict-only.
Rationale (per CLAUDE.md, "Prefer obvious failures over silent
acceptance of wrong-shaped input"): the earlier attempt to "tolerate
scalar 'values' on no-variable formulas" papered over the type
mismatch instead of surfacing it. A user who passes the wrong shape
should learn at the call site, not via a `TypeError: 'int' object is
not iterable` four frames deep in the wrapper or — worse — a wrong
numeric result.
Migration:
# Before
solver = Solver("2*asin(x)", 32)
solver(1, format_digits=28)
# After
solver({"x": "1"}, format_digits=28)
Suggested for the next major release (5.0). values=None continues to
mean "no bindings" — empty dict passes through as before.
Test file renamed from test_solver_scalar_no_vars.py to
test_solver_rejects_non_mapping_values.py to match the new behavior;
existing tests in test_simple.py::test_solver_digits that used the
scalar shortcut are updated to dict form.
See ai/improvements_2026-05-09.md item #2 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
force-pushed
the
fix/02-solver-rejects-scalar-no-vars
branch
from
337a146 to
e88e78f
Compare
hozblok
changed the title
hozblok
added a commit
that referenced
this pull request
May 12, 2026
…cept numeric types, add reverse arithmetic
BREAKING CHANGE: rework of the Number constructor and the four
arithmetic surfaces (forward, reverse, comparison, hashing). Four
pieces, all in src/formula/formula.py:
1. Number.__init__ no longer takes `imaginary_unit` or `case_insensitive`.
Numbers built from Number always use the default complex unit "i"
and case-sensitive parsing. Internal Solvers (in __make_operation,
__make_comparison, __prepare_comparison, __abs__) receive only
`precision` via self.params. Solver itself still accepts the two
kwargs — this change is Number-only.
2. Number(expression) now accepts Union[Number, str, int, float]:
Number(5) # -> Number with expression "5"
Number(3.14) # -> Number with expression "3.14"
Number(Number("x")) # -> unwraps the inner expression
bool is rejected explicitly with a TypeError naming "bool" (without
that check, Number(True) would silently become Number("True")
because bool is an int subclass). Anything else (None, list, dict,
arbitrary objects) raises TypeError naming the offending type and
the accepted set.
3. The reverse-arithmetic dunders (__radd__/__rsub__/__rmul__/
__rtruediv__/__rpow__) drop the prior `Number(str(__value), ...)`
workaround — the constructor now handles type coercion itself,
so `Number(__value, precision=self._precision)` is correct and
reads cleanly. Forward arithmetic and comparison helpers drop the
conditional `__value.expression if isinstance ... else str(...)`
for the same reason.
4. Public type hints on every arithmetic and comparison method widen
from Union[Number, str] to Union[Number, str, int, float] to match
what the constructor now accepts.
Migration:
# Before
Number("1+1*j", imaginary_unit="j")
Number("X+Y", case_insensitive=True)
# After — pre-process the formula text yourself or use Solver
# directly when those flags matter:
Solver("1+1*j", imaginary_unit="j")(...)
Solver("X+Y", case_insensitive=True)(...)
Suggested for the 5.0 milestone alongside #16 (Solver dict-only).
Coordination flag: PR fix/03-number-comparison-honors-params (#17)
currently advertises that Number comparisons honor imaginary_unit and
case_insensitive. After this lands, that part of its premise is gone —
the precision portion of #17 is still real and still wanted. Title
and body of #17 should be scoped down before merge.
Regression coverage:
tests/test_number_constructor_types.py (new) — 10 cases:
int, negative int, float, str (still works), Number unwrap with
independent precision, bool rejected, None rejected, list rejected,
dict rejected, generic object rejected with type-name in message.
tests/test_number_reverse_arithmetic.py — unchanged from the prior
commit on this branch; still passes against the new constructor.
Full pytest suite: 333/333 passing.
See ai/improvements_2026-05-09.md item #4 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
hozblok
added a commit
that referenced
this pull request
May 19, 2026
…cept numeric types, add reverse arithmetic
BREAKING CHANGE: rework of the Number constructor and the four
arithmetic surfaces (forward, reverse, comparison, hashing). Four
pieces, all in src/formula/formula.py:
1. Number.__init__ no longer takes `imaginary_unit` or `case_insensitive`.
Numbers built from Number always use the default complex unit "i"
and case-sensitive parsing. Internal Solvers (in __make_operation,
__make_comparison, __prepare_comparison, __abs__) receive only
`precision` via self.params. Solver itself still accepts the two
kwargs — this change is Number-only.
2. Number(expression) now accepts Union[Number, str, int, float]:
Number(5) # -> Number with expression "5"
Number(3.14) # -> Number with expression "3.14"
Number(Number("x")) # -> unwraps the inner expression
bool is rejected explicitly with a TypeError naming "bool" (without
that check, Number(True) would silently become Number("True")
because bool is an int subclass). Anything else (None, list, dict,
arbitrary objects) raises TypeError naming the offending type and
the accepted set.
3. The reverse-arithmetic dunders (__radd__/__rsub__/__rmul__/
__rtruediv__/__rpow__) drop the prior `Number(str(__value), ...)`
workaround — the constructor now handles type coercion itself,
so `Number(__value, precision=self._precision)` is correct and
reads cleanly. Forward arithmetic and comparison helpers drop the
conditional `__value.expression if isinstance ... else str(...)`
for the same reason.
4. Public type hints on every arithmetic and comparison method widen
from Union[Number, str] to Union[Number, str, int, float] to match
what the constructor now accepts.
Migration:
# Before
Number("1+1*j", imaginary_unit="j")
Number("X+Y", case_insensitive=True)
# After — pre-process the formula text yourself or use Solver
# directly when those flags matter:
Solver("1+1*j", imaginary_unit="j")(...)
Solver("X+Y", case_insensitive=True)(...)
Suggested for the 5.0 milestone alongside #16 (Solver dict-only).
Coordination flag: PR fix/03-number-comparison-honors-params (#17)
currently advertises that Number comparisons honor imaginary_unit and
case_insensitive. After this lands, that part of its premise is gone —
the precision portion of #17 is still real and still wanted. Title
and body of #17 should be scoped down before merge.
Regression coverage:
tests/test_number_constructor_types.py (new) — 10 cases:
int, negative int, float, str (still works), Number unwrap with
independent precision, bool rejected, None rejected, list rejected,
dict rejected, generic object rejected with type-name in message.
tests/test_number_reverse_arithmetic.py — unchanged from the prior
commit on this branch; still passes against the new constructor.
Full pytest suite: 333/333 passing.
See ai/improvements_2026-05-09.md item #4 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hozblok
added a commit
that referenced
this pull request
May 19, 2026
…cept numeric types, add reverse arithmetic
BREAKING CHANGE: rework of the Number constructor and the four
arithmetic surfaces (forward, reverse, comparison, hashing). Four
pieces, all in src/formula/formula.py:
1. Number.__init__ no longer takes `imaginary_unit` or `case_insensitive`.
Numbers built from Number always use the default complex unit "i"
and case-sensitive parsing. Internal Solvers (in __make_operation,
__make_comparison, __prepare_comparison, __abs__) receive only
`precision` via self.params. Solver itself still accepts the two
kwargs — this change is Number-only.
2. Number(expression) now accepts Union[Number, str, int, float]:
Number(5) # -> Number with expression "5"
Number(3.14) # -> Number with expression "3.14"
Number(Number("x")) # -> unwraps the inner expression
bool is rejected explicitly with a TypeError naming "bool" (without
that check, Number(True) would silently become Number("True")
because bool is an int subclass). Anything else (None, list, dict,
arbitrary objects) raises TypeError naming the offending type and
the accepted set.
3. The reverse-arithmetic dunders (__radd__/__rsub__/__rmul__/
__rtruediv__/__rpow__) drop the prior `Number(str(__value), ...)`
workaround — the constructor now handles type coercion itself,
so `Number(__value, precision=self._precision)` is correct and
reads cleanly. Forward arithmetic and comparison helpers drop the
conditional `__value.expression if isinstance ... else str(...)`
for the same reason.
4. Public type hints on every arithmetic and comparison method widen
from Union[Number, str] to Union[Number, str, int, float] to match
what the constructor now accepts.
Migration:
# Before
Number("1+1*j", imaginary_unit="j")
Number("X+Y", case_insensitive=True)
# After — pre-process the formula text yourself or use Solver
# directly when those flags matter:
Solver("1+1*j", imaginary_unit="j")(...)
Solver("X+Y", case_insensitive=True)(...)
Suggested for the 5.0 milestone alongside #16 (Solver dict-only).
Coordination flag: PR fix/03-number-comparison-honors-params (#17)
currently advertises that Number comparisons honor imaginary_unit and
case_insensitive. After this lands, that part of its premise is gone —
the precision portion of #17 is still real and still wanted. Title
and body of #17 should be scoped down before merge.
Regression coverage:
tests/test_number_constructor_types.py (new) — 10 cases:
int, negative int, float, str (still works), Number unwrap with
independent precision, bool rejected, None rejected, list rejected,
dict rejected, generic object rejected with type-name in message.
tests/test_number_reverse_arithmetic.py — unchanged from the prior
commit on this branch; still passes against the new constructor.
Full pytest suite: 333/333 passing.
See ai/improvements_2026-05-09.md item #4 and CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has been amended (force-push of the head branch) to a different fix shape. Original commit history is gone; the diff below is the single commit
e88e78fdnow onfix/02-solver-rejects-scalar-no-vars.Closes item #2 from
ai/improvements_2026-05-09.md.The original bug
Solver("2+2")(values=5)crashed withTypeError: 'int' object is not iterable. The wrapper kept the scalar invariables_to_valuesand the downstream stringification loop iterated over it as if it were a dict. The error message didn't name the parameter, didn't name the offending type, and pointed four frames deep into the wrapper instead of at the call site.The first attempt (rejected)
The first version of this PR silently swallowed the scalar when the formula had no variables (
variables_to_values = {}instead of crashing). It addressed the symptom but encoded the silent-acceptance pattern: a user mistake produced no signal at all, and any future caller that did mean something with that argument would get a wrong numeric answer instead of an error.Why the approach changed
The project's
CLAUDE.md(#35) makes the rule explicit: prefer obvious failures over silent acceptance of wrong-shaped input. A cleanTypeErrorat the boundary names the parameter and the offending type; a silent coercion turns a one-line user mistake into a latent bug.The new behavior
values=None→ empty bindings (unchanged).valuesis aMapping→ used as before.valuesis anything else →TypeError("Solver.__call__ values must be a Mapping; got <type_name>").The previous 1-variable scalar-coercion shortcut is also removed — the API is now consistently dict-only.
Solver("2*asin(x)", 32)(1)used to be a documented shorthand forsolver({"x": "1"}); it now raisesTypeError.This is a behaviour change for callers who relied on the scalar shortcut. The note on the commit is
fix!:(conventional-commits breaking-change marker). Migration is mechanical:Tests
tests/test_solver_scalar_no_vars.py(the file added in the original commit) is deleted — its premise was the silent-tolerate behavior that's now gone.tests/test_solver_rejects_non_mapping_values.pyis added with 8 tests: scalar int (no-var and 1-var formulas), scalar str, scalar float, list, error-message contains the offending type name,values=Nonestill works, dict still works.tests/test_simple.py::test_solver_digitsis updated to use dict form instead of the scalar shortcut. 18 assertion lines touched; nothing else in that test file changes.Full pytest suite: 324/324.
Depends on
Nothing strictly — but the rationale section links to #35 (
docs/add-claude-md-engineering-guidance) which introduces the engineering principle this PR follows. Reviewing them together makes the trail clearer.