Conversation
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thank you, this is so much more elegant than my very crude and churn-heavy py_run_bound!
I guess this basically makes the rest of #3716 ready to throw away that this point? It's now got substantial diff which will never be merged.
| use ::std::option::Option::*; | ||
| if let ::std::result::Result::Err(e) = $py.run($code, None, Some($dict)) { | ||
| use $crate::PyNativeType; | ||
| if let ::std::result::Result::Err(e) = $py.run_bound($code, None, Some(&$dict.as_borrowed())) { |
There was a problem hiding this comment.
Haha using .as_borrowed() here is an incredibly effective way to get it to compile either way around 😂
| ($py:expr, *$dict:expr, $code:expr) => {{ | ||
| use ::std::option::Option::*; | ||
| if let ::std::result::Result::Err(e) = $py.run($code, None, Some($dict)) { | ||
| use $crate::PyNativeType; |
There was a problem hiding this comment.
I'm a little surprised that this doesn't need an #[allow(unused_imports)] 🤔. Eh, we can add it later if it becomes a problem.
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
|
Fixed the conflict
If you prefer to close that one and merge the |
CodSpeed Performance ReportMerging #3816 will degrade performances by 11.36%Comparing Summary
Benchmarks breakdown
|
👍 let's do that, thank you for taking that one over and doing a much better job of it than I did! |
2 participants
Part of #3684, split from #3716 and followup to #3806
This converts to the
Python::runfunction to the new API. For easier reviewing this focuses purely onrun/run_boundand leaves thePyDictmethods for followup PRs.This also changes the
py_runmacro in a way, which allows it to consume both (gil-ref andBound) APIs. So there is no need to introduce apy_run_bound.