-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[mypyc] feat: ForFilter generator helper for builtins.filter
#19643
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
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
Just realized I can extend this class to make it work for both builtins.filter and itertools.filterfalse with only 4 more LOC I wouldn't think to make a primitive for iteration solely over itertools.filterfalse, but considering it would share the same object and only require one more boolean attribute, I think it makes sense to support here. What do you guys think? |
for more information, see https://pre-commit.ci
This reverts commit c9680dc.
for more information, see https://pre-commit.ci
builtins.filter
|
I went ahead and implemented the above on a new branch here, we can merge it into this branch if we decide its worth special casing. I'd say it probably is, considering the code change is quite minimal change once we already have |
|
I don't yet think the failing docs workflow is related to this PR, given that I only added a single bullet point to the native_operations.rst, though can't say for sure |
|
Thanks for the feedbacks. I have a few busy days ahead of me so I'll have to table these for now, but can try to get them all fixed up next week. |
for more information, see https://pre-commit.ci
|
Some tests are failing. Do yo need help with the failures? |
|
Yeah I could definitely use some help with this one, I'm not sure which recent change(s) caused this err nor what to do about it Is this just another typeshed thing? I also have one open question above from one of your review comments. I just tagged you in it to notify. |
oh, whoops! Co-authored-by: Brian Schubert <[email protected]>
|
Okay I've committed that change, once the tests finish the remaining IR errors should be related to the |
mypyc/irbuild/for_helpers.py
Outdated
| # from mypyc.irbuild.expression import transform_call_expr | ||
|
|
||
| # result = transform_call_expr(builder, fake_call_expr) | ||
| result = builder.accept(fake_call_expr) |
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.
I'm reverting this to its original state so the tests can turn green again. You'll see this as part of your review anyway.
This PR adds a
ForFilterhelper class forforloops overbuiltins.filterwhich allows mypyc to maintain control of the function calling, so it can call native functions and use primitive ops, and to optimizefilter's boolean check based onfunc's return value type