-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat[next]: new domain slice syntax #1453
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have a few comments to improve the structure of the code.
tests/next_tests/unit_tests/embedded_tests/test_nd_array_field.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Hannes Vogt <[email protected]>
Co-authored-by: Hannes Vogt <[email protected]>
Co-authored-by: Hannes Vogt <[email protected]>
Co-authored-by: Hannes Vogt <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to fix the typing for AbsoluteIndexElement, by adding slice to it. The other changes are proposal to improve readability.
Co-authored-by: Hannes Vogt <[email protected]>
Co-authored-by: Hannes Vogt <[email protected]>
src/gt4py/next/embedded/common.py
Outdated
return new_index | ||
|
||
|
||
def _named_slice_to_named_range(idx: common.NamedSlice) -> common.NamedRange | common.NamedSlice: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def _named_slice_to_named_range(idx: common.NamedSlice) -> common.NamedRange | common.NamedSlice: | |
def _named_slice_to_named_range(idx: common.NamedSlice) -> common.NamedRange: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically you could just be returning idx without any changes, so you do need common.NamedSlice
in the return type
Co-authored-by: Hannes Vogt <[email protected]>
Co-authored-by: Hannes Vogt <[email protected]>
New domain slice syntax: f[I(-1):I(5)].
E.g. going from:
To: