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
webassembly/objpyproxy: Implement JS iterator protocol for Py iterables. #14435
webassembly/objpyproxy: Implement JS iterator protocol for Py iterables. #14435
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14435 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21204 21204
=======================================
Hits 20864 20864
Misses 340 340 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Damien George <damien@micropython.org>
2ec5a87
to
cb71a7d
Compare
Code size report:
|
68e3f38
to
ac70b1e
Compare
ports/webassembly/objpyproxy.js
Outdated
["number"], | ||
[target._ref], | ||
); | ||
return () => ({ |
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.
nit: this "screams" for a generator to me ... or better, that's usually how iterators are implemented in JS:
return function* () {
while (true) {
const value = Module._malloc(3 * 4);
const valid = Module.ccall(
"proxy_c_to_js_iternext",
"number",
["number", "pointer"],
[iter_ref, value],
);
if (valid) {
yield proxy_convert_mp_to_js_obj_jsside_with_free(value);
}
else {
Module._free(value);
break;
}
}
};
Maybe it doesn't save much typing but at least it grants that the shape of the object is consistent, that is { done: true, value: void 0 }
or { done: false, value: ... }
.
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! I was hoping you would tell me the "right" way to do this 😄
Now updated as suggested.
This allows using JavaScript for..of on Python iterables. Signed-off-by: Damien George <damien@micropython.org>
ac70b1e
to
c056840
Compare
virtually approved 👍 please remember to push incremental updates on npm for quicker feedback in case something ain't fully right (but to me this looks right) 🙏 |
This allows using JavaScript for..of on Python iterables.