-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Support index is list bool for getitem in static mode #33298
Conversation
✅ This PR's description meets the template requirements! |
Thanks for your contribution! |
2b693f0
to
48cceb8
Compare
new_slice_item = [] | ||
for idx, ele in enumerate(slice_item): | ||
if not isinstance(ele, bool): | ||
raise TypeError("Only support bool in list.") |
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.
Here the TypeError should be "Mixed bool index with other types is not supported"
"Only support bool in list." may confuse users that we can only use boolean index.
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.
Thanks, done.
if not isinstance(i, int): | ||
raise TypeError("Only support int value in list") | ||
if not isinstance(i, (int, bool)): | ||
raise TypeError("Only support int or bool in list.") |
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.
"Only support int or bool in list" -> "Only support int or bool in index list"
Otherwise users may be confused about the list thing, them may not know we refer to the index list.
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.
Thanks, done.
48cceb8
to
6b366f9
Compare
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.
LGTM
…ist/Tensor/None/Ellipsis/bool (#33528) * [cherry-pick 2.1] Polish code for _getitem_impl_ (#32868) * [cherry-pick] Polish code for setitem and getitem (#32911) * [slice getitem] Support getitem idx is Tensor or List (#33000) * [getitem] Support index is None for getitem in static mode (#33001) * [Static getitem] Support static Variable getitem for Ellipsis index (#32876) * [static getitem]Support index is list bool for getitem in static mode (#33298)
PR types
New features
PR changes
APIs
Describe
As the title
For example: