Skip to content

fix: enforce the two rules a named offset or width was always said to obey - #61

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

FBumann merged 1 commit into
fix/window-partition-mainfrom
fix/window-width-rules

Conversation

@FBumann

@FBumann FBumann commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Closes #58. Stacked on #59 → #56 — base is fix/window-partition.

What was wrong

The docs state two rules for a named width, both as load errors: it is integral, and it does not span the dimension being summed over. Neither was checked anywhere. within appeared in operators.py and a docstring, and in no pass.

A dtype: float parameter declared over the very axis being summed loaded clean and rendered as:

$$\sum_{t' \in \mathcal{T} ,:, 0 \le t - t' < w} x_{t'}$$

which reads as though w were constant along that axis. On a page whose subject is what a file means, typeset math that claims something the model doesn't say is the worst way to be wrong.

Scope: it was wider than the issue said

shift's offset= documents the same two rules in the same words, and they were unchecked too. Enforcing them for within= while offset= next door still accepted a spanning float would be half a fix, so this is one helper over all three operators.

I audited every documented load error for a named amount. Before this PR, all seven were dead:

rule operator status
integral shift(offset=p) fixed here
does not span the axis walked shift(offset=p) fixed here
integral sum_back/sum_forward within=p fixed here
does not span the axis summed sum_back/sum_forward within=p fixed here
varies only over readable dims shift(offset=p) left — see #62
a named offset requires edge= shift(offset=p) left — see #62
a negative named offset is refused shift(offset=-p) left — see #62, crashes

The last one matters most and is why #62 exists: offset=-lead loads clean and then raises a bare AssertionError out of walk.py, which is precisely the rule that file's docstring says it relies on.

Where the checks live

In the dim pass, because that is where the schema is in hand — a parameter's dtype and its dims come off one declaration, and splitting a documented pair across two passes would give one rule two voices. resolution.py carries dtypes but not parameter dims, so it could enforce one half and not the other.

A literal needs no check at all: it parses as a number, and a number carries no dims.

Review notes

  • Error wording names the rewrite, as the neighbouring rules do — the integral one points at dtype: int "which binds only an integer column, so a fractional width has nowhere to arrive from", and the spanning one says what the operator stops being.
  • Five rejection cases and three positive inference cases in test_dimensions.py; the fixture gains spinup (legal) and horizon (spans the axis) with a comment saying why.
  • No docs changes — the rules were already documented. That was the problem.
  • 446 passing, lint clean.

🤖 Generated with Claude Code

@read-the-docs-community

read-the-docs-community Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

… obey

The docs state both as load errors --- a named amount is integral, and it
does not span the axis its operator walks --- and neither was checked
anywhere. A parameter declared `dtype: float` over the very dimension
being summed passed straight through and rendered as `0 <= t - t' < w`,
which reads as a constant along that axis. The typeset math claimed
something the model did not say, which on a page whose subject is what a
file means is the worst way to be wrong.

Both rules hold of `shift`'s `offset=` in the same words, and that was
unchecked too, so the fix is one helper over both: they are the same
rule about the same kind of argument, and enforcing it for a width while
the offset beside it still accepted a spanning float would be half a
fix.

They live in the dim pass because that is where the schema is in hand
--- a parameter's `dtype` and its `dims` come off one declaration, and
splitting a documented pair across two passes gives it two voices. A
literal needs no check: it parses as a number, and a number carries no
dims.

Closes #58

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@FBumann
FBumann force-pushed the fix/window-width-rules branch from 9dc44bc to 797be03 Compare August 25, 2026 05:40
@FBumann
FBumann changed the base branch from fix/window-partition to fix/window-partition-main August 25, 2026 05:40
@FBumann
FBumann marked this pull request as ready for review August 25, 2026 05:48
@FBumann
FBumann requested a review from brynpickering as a code owner August 25, 2026 05:48
@FBumann
FBumann merged commit 5bd92dc into main Aug 25, 2026
5 of 6 checks passed
@FBumann
FBumann deleted the fix/window-width-rules branch September 9, 2026 06:45
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.

Both documented within= rules are load errors in the docs and no-ops in the loader

1 participant