[stdlib] Parameterize __getitem__ and __setitem__ in stdlib types #2384
Conversation
bgreni
changed the title
bgreni
force-pushed
the
parameterized-indexing
branch
from
c930369
to
94084a3
Compare
stdlib/src/builtin/simd.mojo
Outdated
| Returns: | ||
| The value as an integer | ||
| """ | ||
| constrained[type.is_integral(), "expected integral type"]() |
There was a problem hiding this comment.
This may not be a good idea, for SIMD at least. This way, SIMD[bool, 1] will be cast to a Int but we might want to index with the original mask (which is size 1).
There was a problem hiding this comment.
This may not be a good idea, for
SIMDat least. This way,SIMD[bool, 1]will be cast to aIntbut we might want to index with the original mask (which is size 1).
Now that you mention it Scalar[bool] actually won't be accepted here, which is inconsistent as I've currently given Bool the Indexer trait, but I was hoping for feedback from the team on whether that was acceptable or not.
We need some support for this in SIMD, however, since it would be expected that one could use any of the integral scalars to index the stdlib containers.
There was a problem hiding this comment.
I just think we need a more complete design before adding the support to stdlib.
There was a problem hiding this comment.
__index__ is an established pattern in Python, and so is indexing with bool so it seems reasonable to me to implement it this way for Mojo.
bgreni
force-pushed
the
parameterized-indexing
branch
3 times, most recently
from
c4962ae
to
8902489
Compare
bgreni
force-pushed
the
parameterized-indexing
branch
5 times, most recently
from
be29ebc
to
5c0c215
Compare
bgreni
force-pushed
the
parameterized-indexing
branch
5 times, most recently
from
af3b58f
to
aa76d86
Compare
bgreni
changed the title
bgreni
force-pushed
the
parameterized-indexing
branch
from
aa76d86
to
06e9052
Compare
When indexing stdlib containers we should accept a generic type that calls on the __index__ method to allow types other than Int to be used but doesn't allow Intable types that should not be used for such purposes (such as Float) Signed-off-by: Brian Grenier <grenierb96@gmail.com>
bgreni
force-pushed
the
parameterized-indexing
branch
from
06e9052
to
d4bd2d8
Compare
Fixes #2337
As mentioned in the referenced issue, the
__index__()method allows types other than the baseIntto be used when indexing containers without allowing inappropriate types that happen to implementIntablefrom being used (such as float point scalars).Introduce the
Indexertrait that requires the type to implement the__index__()method, and change all instances of__getitem__and__setitem__to accept any type that has theIndexertrait.