Skip to content

refactor: tighten prepare_expression — drop stale TODOs, expose original input#42

Draft
hozblok wants to merge 1 commit into
masterfrom
refactor/prepare-expression-tighten
Draft

refactor: tighten prepare_expression — drop stale TODOs, expose original input#42
hozblok wants to merge 1 commit into
masterfrom
refactor/prepare-expression-tighten

Conversation

@hozblok
Copy link
Copy Markdown
Owner

@hozblok hozblok commented May 19, 2026

Closes two TODOs in Formula::prepare_expression:

TODO Resolution
csformula.cpp:185 // TODO check for forbidden symbols for complex: < or >, <= or >=. Already implemented in the same function (lines 217-222 throw on < / > when is_complex_, and that catches <= / >= too via substring match). Just delete the stale comment.
csformula.cpp:195 // TODO cannot give back to user the original expression? Resolved: add a parallel expression_original_ field, stash the unmolested user input before whitespace stripping / case lowering, expose via get_original_expression() + original_expression property.

Changes

  • src/cpp/csformula/csformula.hpp — new private std::string expression_original_;, new public get_original_expression().
  • src/cpp/csformula/csformula.cppprepare_expression stashes the input into expression_original_ before any normalization. Both TODO comments deleted.
  • src/cpp/main.cpp — pybind11 binding gains a get_original_expression method and an original_expression read-only property on the Formula class.

Tests

tests/test_prepare_expression.py — 12 cases:

  • Forbidden-symbol rejection on complex — parametrized over <, >, <=, >= on both sides of i (6 cases). All raise ValueError with the existing message.
  • < allowed on real expressions — no i → real path, comparison is a valid binary op.
  • get_original_expression preserves case"X + Y" with case_insensitive=Trueexpression_ lowered to "x+y", expression_original_ keeps "X + Y".
  • Preserves whitespace" x + y " keeps spaces in the original even though expression_ strips them.
  • original_expression property accessor — works the same as get_original_expression().
  • Equal to get_expression() when no normalization happened.
  • Updates on set_expression() — both stay in sync.

Full suite 400/400 (+12 new), 3 xfailed unchanged.

…nal input

Two TODO comments in prepare_expression that were never resolved:

  csformula.cpp:185  // TODO check for forbidden symbols for complex
  csformula.cpp:195  // TODO cannot give back to user the original expression?

The first was already implemented elsewhere in the same function
(lines 217-222 throw on '<' / '>' when is_complex_, and that catches
'<=' / '>=' too via substring match) — just delete the now-stale
comment.

The second is now resolved: add a parallel `expression_original_`
private field, stash the unmolested user input in prepare_expression
before whitespace stripping or case lowering, expose via a new
`get_original_expression()` getter plus an `original_expression`
read-only property on the pybind11 binding.

Regression test in tests/test_prepare_expression.py:
- forbidden symbols on complex throw with the existing message (six
  cases covering '<', '>', '<=', '>=' on both sides of 'i')
- '<' allowed on real-only expressions
- get_original_expression preserves case and whitespace
- original_expression property accessor works
- updates correctly on set_expression

Full suite 400/400 (+12 new), 3 xfailed unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant