Split Map2d.iter_data overloads by direction literal - #677
Merged
Conversation
The iter_data overloads came in pairs per arity: one keyed on Literal[IterDirection.Rows] returning row-major tuples, and a general one accepting any IterDirection and returning column-major tuples, the general signature declared first. Overload resolution picks the first match, and Literal[IterDirection.Rows] is assignable to IterDirection, so the general signature swallowed every call that named IterDirection.Rows explicitly and typed its result as column-major - the transpose of what the implementation yields. Merely declaring the literal signature first moves that unsoundness rather than removing it: the general signature then still promises column-major for an argument that may be Rows at run time, which mypy reports as an overload-overlap error. Give each arity three signatures instead: Literal[IterDirection.Rows], Literal[IterDirection.Columns], and a general IterDirection returning the union of both shapes. The two literal signatures are disjoint, and each returns a subtype of the union the general signature promises, so no unsafe overlap is left. The union is also the honest answer for a caller whose direction is only known as an IterDirection, since iter_data branches on that value at run time. Behaviour is unchanged and no call site needed adjusting.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The overload restructuring matches the runtime branching behavior, removes the documented typing unsoundness, and doesn’t alter runtime logic.
Pull request overview
This PR fixes type-checking unsoundness in Map2d.iter_data overloads by making the overload set disjoint for Rows vs Columns literals and making the non-literal overload return the honest union shape that matches the runtime branch on direction.
Changes:
- Split
iter_dataoverloads (for each arity) intoLiteral[IterDirection.Rows],Literal[IterDirection.Columns], and a generalIterDirectionoverload returning a union of both iterator shapes. - Ensured calls explicitly passing
IterDirection.Rowsare typed as row-major (matching implementation), rather than being swallowed by a broader overload.
File summaries
| File | Description |
|---|---|
| solvers/python/src/aoc/tooling/map.py | Reworks iter_data overload signatures to avoid overload overlap and align static return types with runtime behavior. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
The iter_data overloads came in pairs per arity: one keyed on Literal[IterDirection.Rows] returning row-major tuples, and a general one accepting any IterDirection and returning column-major tuples, the general signature declared first.
Overload resolution picks the first match, and Literal[IterDirection.Rows] is assignable to IterDirection, so the general signature swallowed every call that named IterDirection.Rows explicitly and typed its result as column-major - the transpose of what the implementation yields. Merely declaring the literal signature first moves that unsoundness rather than removing it: the general signature then still promises column-major for an argument that may be Rows at run time, which mypy reports as an overload-overlap error.
Give each arity three signatures instead: Literal[IterDirection.Rows], Literal[IterDirection.Columns], and a general IterDirection returning the union of both shapes. The two literal signatures are disjoint, and each returns a subtype of the union the general signature promises, so no unsafe overlap is left. The union is also the honest answer for a caller whose direction is only known as an IterDirection, since iter_data branches on that value at run time.
Behaviour is unchanged and no call site needed adjusting.