Repository navigation
fix: reject variable names SymPy cannot parse before the search starts - #1418
Merged
MilesCranmer merged 2 commits intoOct 8, 2026
Merged
Conversation
Equations are converted to SymPy by parsing them as Python expressions, but variable names were only checked against a character regex and against existing function names. Names that pass those checks yet are not valid Python identifiers ran the whole search, and then `fit` failed in `get_hof` when parsing the hall of fame, e.g. with `SympifyError: could not parse '0.0659 / (x₁ + 0.86)'`: - Python keywords (`lambda`, a common physics symbol, or `in`); - names starting with a digit (`1x`); - names with subscript digits (`x₁`), which the regex allows explicitly. A variable named like a key of `extra_sympy_mappings` was also accepted, although the parser then resolves the name to the function. Check all of these up front for expression specs that export to SymPy, so the error is raised before any time is spent searching. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reword the error for names that are not valid non-keyword Python identifiers, add a rename hint, document the constraint in the fit docstring, and test a numeric-looking name such as "12", which master silently turns into a constant. Co-authored-by: Miles Cranmer <miles.cranmer@gmail.com>
MilesCranmer
approved these changes
Oct 8, 2026
MilesCranmer
left a comment
MilesCranmer
left a comment
Member
There was a problem hiding this comment.
This looks like a helpful error indeed. Thanks
MilesCranmer
enabled auto-merge (squash)
October 8, 2026 21:30
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
3 participants
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.
Summary
Equations are converted to SymPy by parsing them as Python expressions, but variable names were only checked against a character regex and existing function names. Some names that passed those checks break that parsing:
12(e.g. wavelength column headers), are parsed as constants, so predictions were silently wrong.lambdaorin, names starting with a digit such as1x, and names with subscript digits such asx₁ran the entire search, andfitthen failed while parsing the hall of fame, losing the in-memory result.extra_sympy_mappingsresolved to the function instead of the variable.For expression specs that export to SymPy,
fitnow rejects these names before the search starts, with a hint on how to rename them. Specs without SymPy export, such asTemplateExpressionSpec, are unaffected. Thevariable_namesdocstring lists the rules.Tests
test_variable_names_sympy_cannot_parse_fail_before_searchcoverslambda,in,1x,12,x₁, and a name clashing withextra_sympy_mappings.