feat[cartesian]: Remove redundant region syntax#2554
Merged
twicki merged 7 commits intoGridTools:mainfrom Mar 27, 2026
Merged
Conversation
romanc
reviewed
Mar 26, 2026
Contributor
romanc
left a comment
romanc
left a comment
There was a problem hiding this comment.
Some remaining test failures, otherwise looking good to me. I guess there are a couple tests remaining that still use the now forbidden syntax.
| ): | ||
| raise GTScriptSyntaxError( | ||
| "Invalid horizontal range specification:" | ||
| f"Expected specification {self.axis_name}[0] or {self.axis_name}[-1]." |
Contributor
There was a problem hiding this comment.
I don't understand why there this exception to allow I[-1]? What is different about -1?
Contributor
Author
There was a problem hiding this comment.
so the goal is to allow to access both ends of the domain. The way this was previously done was with "standard" slicing notation, so I[-1] is the end of the I-domain. That way you can offset from both sides without having to know the domain-size.
And because we're still in AST-land here, -1 is not resolved into a negative constant but it is a unary-up with the value 1.
romanc
approved these changes
Mar 26, 2026
| ): | ||
| raise GTScriptSyntaxError( | ||
| "Invalid horizontal range specification:" | ||
| f"Expected specification {self.axis_name}[0] or {self.axis_name}[-1]." |
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
2 participants
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.
Series on interval-parsing (3/3)
This PR is one in a series to simplify interval parsing in the front-end.
This entire series was triggered, because runtime-intervals and higher dimensional fields do not interact as intended right now. So here the summary of the slate of work
Current State:
[]-operators did trigger exceptionsTo tackle these problems, the following PR's are up:
2510: split the parser for the horizontal and the vertical and specialize them as appropriate
2553: fix how runtime-intervals interact with higher dimensional fields
current: remove all redundant syntax to go back to a more specific language
Description
The legal syntax for regions was very bloated and allowed for multiple ways to express a single output. This PR trims this down to a more minimalistic approach.
Previously allowed:
I[0:2]I[0] : I[2]I[0] : I[0] + 20:2There is no clean way to completely avoiding the axis-access syntax, as it is required to generate somewhat clean out-of-domain computations to skip by going
I[0]- LARGENUMBER. this is why axis-access is still allowed additionally to plain offset syntax. but only to access the start and end of the domain.So this PR minimizes the syntax down to
I[0] : I[0] + 20:2