-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Convert function, method, builtin_*, frame, and generator to Any payload #642
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
Codecov Report
@@ Coverage Diff @@
## master #642 +/- ##
==========================================
- Coverage 40.81% 40.78% -0.03%
==========================================
Files 76 77 +1
Lines 17345 17383 +38
Branches 4474 4484 +10
==========================================
+ Hits 7079 7090 +11
- Misses 8366 8396 +30
+ Partials 1900 1897 -3
Continue to review full report at Codecov.
|
trace!("invoke __call__ for: {:?}", payload); | ||
self.call_method(&func_ref, "__call__", args) | ||
} | ||
if let Some(PyFunction { |
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.
Why is a match statement no option here? I feel match is better than if statements with returns.
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 can get rid of the explicit returns, but a match
won't work here because each payload
call is generic over the specific payload type.
(I do have some ideas on how to make this kind of thing cleaner and more efficient, but they're not fully baked yet.)
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 think we could use a macro, something like match_payload!
, which would look like:
let match_res = match_payload!(pyobjref, {
objint::PyInt(int) => do_thing(int.value),
objfloat::PyFloat(objfloat::PyFloat { value }) => do_float_thing(value),
});
I had some minor comment, but it looks good! |
Getting so close now!