Skip to content

fix: let sum_back stop at each group's edge, as its checks already assumed - #65

Merged
FBumann merged 1 commit into
mainfrom
fix/window-partition-main
Aug 25, 2026
Merged

FBumann merged 1 commit into
mainfrom
fix/window-partition-main

Conversation

@FBumann

@FBumann FBumann commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What this changes

sum_back accepts by=<lookup>, so a window stops at each group's edge instead of walking across it.

  • operators.py — declares the kwarg on the sum_back builtin (lookup_kwargs, optional_kwargs) and shows it in the usage string.
  • typesetting/walk.py — a new _group() renders the partition as the superscript its operator carries, and the window's lag passes it through.
  • examples/operators/sum_back_partitioned.yaml, one golden constraint, one tools/spec_math.py row; docs and goldens regenerated from those.

Extracted from #59, which does the same for both windows. The sum_forward half stays there, since that operator only exists in #56.

Why

Two of the three places that describe this rule already agreed it held. The comment beside BUILTINS says by= partitions the axis for "shift and sum_back", and _dims_call already runs every partition check for both names — one lookup only, and it must be over the walked dim. Only BUILTINS never declared the kwarg, and that is the one that gates parsing, so the rule they described was unreachable:

before:  SchemaError: sum_back() expects
         sum_back(<expr>, over=<dim>, within=<n|parameter>[, edge='wrap'])
after:   accepted

Declaring the kwarg is the whole fix. The typesetting side needed the group as a superscript on the operator, which _Step.within and translation(step, group=...) were already shaped for — a partition rides a window exactly as it rides a leaf translation, and for the same reason: what the group changes is where the axis ends, not which coordinate is being written.

∑_{t' ∈ 𝒯 : 0 ≤ t -^season_of(t) t' < 3} on_{t',g} ≤ units_g   ∀ t ∈ 𝒯, g ∈ 𝒢

This is byte-identical to what #59 renders for the same constraint.


Generated by Claude Code

…sumed

`BUILTINS` never declared `by=` on `sum_back`, so the call was refused at
load — while the operator comment beside it already said `by=` partitions
the axis for "``shift`` and ``sum_back``", and `_dims_call` already ran the
partition checks for both. Two of the three places agreed; the one that
gates parsing did not, so the rule they described was unreachable.

Declaring the kwarg is the whole fix. The typesetter needed the group as
the superscript its operator carries, which `_Step.within` and
`translation(step, group=...)` were already shaped for — a partition rides
a window exactly as it does a leaf translation, and for the same reason:
what the group changes is where the axis ends, not which coordinate is
being written.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018SeNnVvhYnokz8C37o7ejW
@FBumann
FBumann requested a review from brynpickering as a code owner August 25, 2026 05:39
@read-the-docs-community

Copy link
Copy Markdown

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.

2 participants