-
Notifications
You must be signed in to change notification settings - Fork 430
Add Python
package module for Python+Chapel interop
#26156
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
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
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.
Read the code, not the tests yet.
modules/packages/Python.chpl
Outdated
init this; | ||
this.fn!.check(); |
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.
Consider invoking the name/object constructor (init#2) to share some code.
modules/packages/Python.chpl
Outdated
proc init(interpreter: borrowed Interpreter, in lambdaFn: string) throws { | ||
this.fnName = "<anon>"; | ||
init this; | ||
this.fn = interpreter.compileLambda(lambdaFn); |
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.
TODO come back to this
modules/packages/Python.chpl
Outdated
var pyArgs: args.size * c_ptr(void); | ||
for param i in 0..#args.size { | ||
pyArgs(i) = interpreter.toPython(args(i)); | ||
} | ||
var pyRes; | ||
if pyArgs.size == 1 then | ||
pyRes = PyObject_CallOneArg(this.obj!.get(), pyArgs(0)); | ||
else | ||
pyRes = PyObject_CallFunctionObjArgs(this.obj!.get(), (...pyArgs), nil); |
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.
This is duplicated; might you be able to share it with the Function
instance? Moreover, aren't functions "just" ClassObjects?
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.
Yes, there is a lot of duplicated code right now between Function and ClassObject. I plan to introduce a class hierarchy to handle this in a future PR
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
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.
Your changes look good and I've read-skimmed the tests, trusting you on those.
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Fixes some new Python interop tests (added in #26156) with COMM!=none. This is caused by a difference between Chapel's stdio and Python's stderr. The difference is that Chapel does some extra work in the multi-locale case to make sure everything is printed on the right locales, whereas Python has no native concept of locality. In this case, this just manifests as changing whether the Python output comes first or last. This PR also switches the interpreter to be explicitly an "isolated" interpreter, to prevent potential issues with Python setting up its stdio. Tested that all Python tests pass with COMM=none and COMM=gasnet [Reviewed by @DanilaFe]
Adds a new package module,
Python
, which supports calling Python code from Chapel.For example, the following Chapel progam use the python interface to
BeautifulSoup
to parse HTMLAs an another example, this simple program compiles an runs a Python lambda from Chapel code
Future work:
Value
so that more implementation can be shared and the Chapel interface can better imitate python.call("__add__", ...)
), but it would be nice to support native operators like+
[Reviewed by @DanilaFe]