You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I want to fix issues #1944, #2046, and #2142. Those are List but the same problem is there for String. I actually have a fix done here with the interesting commit here. The implementation just has some ugliness that needs discussion to resolve.
The gist is that Python swaps the default start and end (with some fiddling for exclusivity at the end point) when step is negative.
Mojo:
vars= String("abcd")
print(s[2::-1[) # prints "dc"
Python3:
s="abcd"print(s[2::-1]) #prints "cba"
To fix this I created a new struct, RawSlice, that takes OptionalReg[Int] for start and end. By preserving the programmers intent to include the end of the range it can correctly handle both unspecified start or end as well as negative start or end. Whichever direction you are slicing in the start index is inclusive while the end index is exclusive. So with step = -1, end = -1 if given by the programmer meanssize - 1 but if the programmer has specified end = None then Slice.end needs to be -1 so that the 0 index value is included in the slice.
My solution works fine but I think requiring people doing custom slicing to know about Slice and RawSlice (or some other awesomely-named struct) is probably confusing. So I feel like there is a cleaner solution.
Simply replacing Slice with RawSlice will need to raise in many uses like __getitem__ and __len__ because the start and end could be None. The error message could be nice since RawSlice has an adjust method that can replace all None or programmer provided negatives with correct indices. So the solution to these errors is call adjust and use the method that raised on the result. But maybe raising could be avoided?
The current Slice, is a lot like a _StridedRange. Slice just adds the traits EqualityComparable and Stringable. Also _StridedRange lacks docstrings since it is underscored. So maybe _StridedRange is all that is needed. But it would need to be made public since it will be used outside of iterating a range.
So I am not happy wth my current solution and the two other possibilities I see don't look obviously great either. And I am not sure any of them amount to a PR that would land. So I thought I would start a discussion and see what others think. It doesn't seem substantive enough for the proposal process.
The core problem of getting slices with negative step to match Python does seem important though.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
I want to fix issues #1944, #2046, and #2142. Those are List but the same problem is there for String. I actually have a fix done here with the interesting commit here. The implementation just has some ugliness that needs discussion to resolve.
The gist is that Python swaps the default start and end (with some fiddling for exclusivity at the end point) when step is negative.
Mojo:
Python3:
To fix this I created a new struct,
RawSlice
, that takesOptionalReg[Int]
forstart
andend
. By preserving the programmers intent to include the end of the range it can correctly handle both unspecifiedstart
orend
as well as negativestart
orend
. Whichever direction you are slicing in the start index is inclusive while the end index is exclusive. So withstep = -1
,end = -1
if given by the programmer meanssize - 1
but if the programmer has specifiedend = None
thenSlice.end
needs to be-1
so that the 0 index value is included in the slice.My solution works fine but I think requiring people doing custom slicing to know about Slice and RawSlice (or some other awesomely-named struct) is probably confusing. So I feel like there is a cleaner solution.
Slice
withRawSlice
will need to raise in many uses like__getitem__
and__len__
because thestart
andend
could be None. The error message could be nice sinceRawSlice
has anadjust
method that can replace all None or programmer provided negatives with correct indices. So the solution to these errors is calladjust
and use the method that raised on the result. But maybe raising could be avoided?Slice
, is a lot like a_StridedRange
.Slice
just adds the traitsEqualityComparable
andStringable
. Also_StridedRange
lacks docstrings since it is underscored. So maybe_StridedRange
is all that is needed. But it would need to be made public since it will be used outside of iterating a range.So I am not happy wth my current solution and the two other possibilities I see don't look obviously great either. And I am not sure any of them amount to a PR that would land. So I thought I would start a discussion and see what others think. It doesn't seem substantive enough for the proposal process.
The core problem of getting slices with negative step to match Python does seem important though.
Beta Was this translation helpful? Give feedback.
All reactions