Repository navigation
fix: enforce the two rules a named offset or width was always said to obey - #61
Merged
Merged
Conversation
… 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
force-pushed
the
fix/window-width-rules
branch
from
August 25, 2026 05:40
9dc44bc to
797be03
Compare
FBumann
changed the base branch from
fix/window-partition
to
fix/window-partition-main
August 25, 2026 05:40
FBumann
marked this pull request as ready for review
August 25, 2026 05:48
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
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.
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.
withinappeared inoperators.pyand a docstring, and in no pass.A
dtype: floatparameter declared over the very axis being summed loaded clean and rendered as:which reads as though
wwere 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'soffset=documents the same two rules in the same words, and they were unchecked too. Enforcing them forwithin=whileoffset=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:
shift(offset=p)shift(offset=p)sum_back/sum_forwardwithin=psum_back/sum_forwardwithin=pshift(offset=p)edge=shift(offset=p)shift(offset=-p)The last one matters most and is why #62 exists:
offset=-leadloads clean and then raises a bareAssertionErrorout ofwalk.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
dtypeand itsdimscome off one declaration, and splitting a documented pair across two passes would give one rule two voices.resolution.pycarries 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
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.test_dimensions.py; the fixture gainsspinup(legal) andhorizon(spans the axis) with a comment saying why.🤖 Generated with Claude Code