Skip to content

Parse data-dict definition expressions - #269

Draft
jat255 wants to merge 1 commit into
mainfrom
jat255/m2-f6hz-expression-parser
Draft

Parse data-dict definition expressions#269
jat255 wants to merge 1 commit into
mainfrom
jat255/m2-f6hz-expression-parser

Conversation

@jat255

@jat255 jat255 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

First of four PRs porting the data-dict definition compiler to Python (kata f6hz, stage 1). Text in, AST out. No type checking, so nothing here produces an export record yet.

_expression.py follows pkg-r/R/definition-expression.R closely enough to diff against it, which is the only practical way to review a hand-port this size. The later PRs are _export.py, _emit_duckdb.py, then _compile.py and the wiring.

The tests assert the AST rather than only the absence of a parse error. A parser that accepts everything and builds the wrong tree passes a "the corpus parses" check and then silently misinforms the type checker built on top of it, so the constructs with dedicated parse functions get their own cases alongside the precedence and associativity rules.

Both shared-corpus tests passed on first run, so I confirmed they can fail: dropping COLUMNS support fails the valid corpus (42 expressions), and accepting a missing operand fails parse.yaml. A third perturbation, removing the trailing-text check, failed neither, because amount + is refused for its missing right operand instead. That is recorded in the commit message so the next reader does not repeat it.

One deliberate departure from R: integer literals are stored as int rather than as the normalized string R keeps to protect 64-bit precision. Python needs no such protection, and the range check against 9223372036854775807 is still there because data-dict's is a Rust i64.

Verification: 406 tests pass, ruff and pyrefly clean on src and tests.

First of four PRs porting the definition compiler (kata f6hz). Text in,
AST out; no types yet, so nothing here produces an export record.

The AST is asserted directly, not just checked for the absence of a parse
error. A parser that accepts everything and builds the wrong tree would
pass a "the corpus parses" check and then silently misinform the type
checker built on top of it. The constructs with dedicated parse functions
(intervals, COLUMNS, CASE, quoted names, field paths) get their own cases,
along with the precedence and associativity rules.

The two corpus tests were confirmed to bite by perturbing the parser:
dropping COLUMNS support fails the valid corpus, and accepting a missing
operand fails parse.yaml. The first perturbation attempted, removing the
trailing-text check, did not fail either test, because `amount +` is
refused for its missing right operand instead.
@jat255
jat255 marked this pull request as draft September 4, 2026 05:49
@jat255 jat255 added this to the py-M2: data layer milestone Sep 4, 2026
@jat255 jat255 added needs-manual-review Agent-created work that needs a human review py Affects the Python implementation labels Sep 4, 2026
@jat255

jat255 commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Manual review completed 2026-09-04 21:29 MDT. Final pass over the parser and its tests: clean (two INFO-level notes only: >4300-digit float literals bypass the positioned error message, and the match_word prefix-restore case in the docstring has no pinning test). CI green across the matrix; scoped pytest + ruff re-run locally. Removing needs-manual-review.

@jat255 jat255 removed the needs-manual-review Agent-created work that needs a human review label Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

py Affects the Python implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant