-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[mypyc] Improve support for frozenset #18571
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
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.
Thanks! Frozensets are likely be used in performance critical code, so having better support for them is helpful. Left a few minor comments, overall looks good.
@@ -0,0 +1,536 @@ | |||
[case testNewFrozenSet] |
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.
The irbuild tests are maybe too verbose -- they may need to be updated frequently as the details of emitter IR change. My suggestion would be to avoid some duplication and to move some thing to run tests if they don't otherwise have test coverage.
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.
Tbh for the first implementation I mostly copied the set
test cases and adjusted them slightly for frozenset
. Your suggestion does make sense though. Moved some of the fromIterable
tests to run_sets.test
.
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.
Yeah some of the existing IR build tests are overly verbose.
1d2a393
to
8edec05
Compare
Add optimized methods for `len` and `contains` on `frozenset` objects.
Add optimized methods for
len
andcontains
onfrozenset
objects.