-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[stdlib] Tuple.__contains__ #2709
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.
Pretty cool, thank you!
@laszlokindrat Yes, sorry, the tests are more complete now, do you see more cases to add ? There is a test failing in |
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
Signed-off-by: rd4com <144297616+rd4com@users.noreply.github.com>
The force-push worked with the rebased branch 🥳 :
Todo:
ℹ️ Note that the rebase is on |
!sync |
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 again! I think this is a useful functionality, and while we can't do it perfectly yet, it does deliver a useful feature, especially for people coming from Python. There are very likely some sharp edges around the current behavior. If you can think of any, please open a PR to document them.
@laszlokindrat , This is great! this is a cool feature, i'd be happy to use it too 😄 . Once auto-dereference lands:
If there is any way you can add this to sort of a todo list to make sure |
The tests should prevent it from being broken. |
✅🟣 This contribution has been merged 🟣✅ Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours. We use Copybara to merge external contributions, click here to learn more. |
Landed in ea528b3! Thank you for your contribution 🎉 |
[External] [stdlib] Tuple.__contains__ Hello, here is an implementation of ```Tuple.__contains__```, it makes it possible to do: ```mojo var x = Tuple(1,2,True) if 1 in x: print("x contains 1") ``` --------- Co-authored-by: rd4com <144297616+rd4com@users.noreply.github.com> Closes #2709 MODULAR_ORIG_COMMIT_REV_ID: ec8cf2aaa0912d06f87c2b2ad481a7aa99b4cc03
[External] [stdlib] Tuple.__contains__ Hello, here is an implementation of ```Tuple.__contains__```, it makes it possible to do: ```mojo var x = Tuple(1,2,True) if 1 in x: print("x contains 1") ``` --------- Co-authored-by: rd4com <144297616+rd4com@users.noreply.github.com> Closes modularml#2709 MODULAR_ORIG_COMMIT_REV_ID: ec8cf2aaa0912d06f87c2b2ad481a7aa99b4cc03
[External] [stdlib] Fix `Tuple.__contains__` Various follow-ups on modularml#2709. This also ensures `Tuple.__contains__` works in the parameter domain. Co-authored-by: rd4com <144297616+rd4com@users.noreply.github.com> Closes modularml#2782 MODULAR_ORIG_COMMIT_REV_ID: 88a3d728bbda276c34ee485d3ee7a57c147ad403
Hello,
here is an implementation of
Tuple.__contains__
,it makes it possible to do:
@laszlokindrat suggested a ping for the PR in #2658