Skip to content

Commit

Permalink
Allow users to omit the return type on some Python calls (#26663)
Browse files Browse the repository at this point in the history
Allows users to omit the return type on a call to Python code when they
don't care what the type is. The defaults to `owned Value`, which is a
wrapper around the underlying type. This `Value` can be captured by
users and later converted to a Chapel type, continue to be used by other
functions as a `Value` type (with no extra overhead), or ignored
all together (in the case a function from python doesn't return
anything).

This significantly cleans up some user-facing Python code

Before this PR
```chapel
doNothing(NoneType);
funcReturnsNothing(NoneType, 1, 2, 3);
var x: owned Value = getAValue(owned Value);
```

After this PR
```chapel
doNothing();
funcReturnsNothing(1, 2, 3);
var x: owned Value = getAValue();
```

- [x] ran `start_test test/library/packages/Python/`

[Reviewed by @lydia-duncan]
  • Loading branch information
jabraham17 authored Feb 6, 2025
2 parents 9af93c5 + 0b7c0ab commit 6eb636d
Show file tree
Hide file tree
Showing 18 changed files with 267 additions and 66 deletions.
96 changes: 67 additions & 29 deletions modules/packages/Python.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
var subInterpreter = new SubInterpreter(interpreter);
var m = new Module(subInterpreter, 'myMod', code);
var hello = new Function(m, 'hello');
hello(NoneType);
hello();
}
}
Expand Down Expand Up @@ -325,8 +325,8 @@
writeln("Let's call some Python!");
IO.stdout.flush(); // flush the Chapel output buffer before calling Python
func(NoneType, "Hello, World!");
func(NoneType, "Goodbye, World!");
func("Hello, World!");
func("Goodbye, World!");
interp.flush(); // flush the Python output buffer before calling Chapel again
writeln("Back to Chapel");
Expand Down Expand Up @@ -1375,7 +1375,14 @@ module Python {
:arg args: The arguments to pass to the callable.
:arg kwargs: The keyword arguments to pass to the callable.
*/
pragma "docs only"
proc this(type retType = owned Value,
const args...,
kwargs:?=none): retType throws
where kwargs.isAssociative() do compilerError("docs only");

pragma "last resort"
@chpldoc.nodoc
proc this(type retType,
const args...,
kwargs:?=none): retType throws
Expand All @@ -1386,20 +1393,40 @@ module Python {
}
pragma "last resort"
@chpldoc.nodoc
proc this(const args...,
kwargs:?=none): owned Value throws
where kwargs.isAssociative() {
var pyArg = this.packTuple((...args));
defer Py_DECREF(pyArg);
return callInternal(owned Value, pyArg, kwargs);
}
pragma "last resort"
@chpldoc.nodoc
proc this(type retType,
kwargs:?=none): retType throws where kwargs.isAssociative() {
var pyArgs = Py_BuildValue("()");
defer Py_DECREF(pyArgs);
return callInternal(retType, pyArgs, kwargs);
}
pragma "last resort"
@chpldoc.nodoc
proc this(kwargs:?=none): owned Value throws where kwargs.isAssociative() {
var pyArgs = Py_BuildValue("()");
defer Py_DECREF(pyArgs);
return callInternal(owned Value, pyArgs, kwargs);
}

@chpldoc.nodoc
proc this(type retType, const args...): retType throws {
var pyArg = this.packTuple((...args));
defer Py_DECREF(pyArg);
return callInternal(retType, pyArg, none);
}
@chpldoc.nodoc
proc this(type retType): retType throws {
proc this(const args...): owned Value throws do
return this(owned Value, (...args));
@chpldoc.nodoc
proc this(type retType = owned Value): retType throws {
var pyRes = PyObject_CallNoArgs(this.get());
this.check();

Expand Down Expand Up @@ -1441,13 +1468,22 @@ module Python {
:arg t: The Chapel type of the value to return.
:arg attr: The name of the attribute/field to access.
*/
pragma "docs only"
proc getAttr(type t=owned Value, attr: string): t throws do
compilerError("docs only");

@chpldoc.nodoc
proc getAttr(type t, attr: string): t throws {
var pyAttr = PyObject_GetAttrString(this.get(), attr.c_str());
interpreter.checkException();

var res = interpreter.fromPython(t, pyAttr);
return res;
}
@chpldoc.nodoc
proc getAttr(attr: string): owned Value throws do
return this.getAttr(owned Value, attr);


/*
Set an attribute/field of this Python object. This is equivalent to
Expand All @@ -1472,6 +1508,11 @@ module Python {
:arg method: The name of the method to call.
:arg args: The arguments to pass to the method.
*/
pragma "docs only"
proc call(type retType = owned Value, method: string, const args...): retType throws do
compilerError("docs only");

@chpldoc.nodoc
proc call(type retType, method: string, const args...): retType throws {
var pyArgs: args.size * PyObjectPtr;
for param i in 0..#args.size {
Expand All @@ -1489,6 +1530,10 @@ module Python {
var res = interpreter.fromPython(retType, pyRes);
return res;
}
@chpldoc.nodoc
proc call(method: string, const args...): owned Value throws do
return this.call(owned Value, method, (...args));

@chpldoc.nodoc
proc call(type retType, method: string): retType throws {
var methodName = interpreter.toPython(method);
Expand All @@ -1500,6 +1545,11 @@ module Python {
var res = interpreter.fromPython(retType, pyRes);
return res;
}
@chpldoc.nodoc
proc call(method: string): owned Value throws do
return this.call(owned Value, method);

// TODO: call should support kwargs
}

/*
Expand Down Expand Up @@ -1623,31 +1673,6 @@ module Python {
obj: PyObjectPtr, isOwned: bool = true) {
super.init(interpreter, obj, isOwned=isOwned);
}

@chpldoc.nodoc
proc newInstance(const args...): PyObjectPtr throws {
var pyArg = packTuple((...args));
defer Py_DECREF(pyArg);
var pyRes = PyObject_Call(this.get(), pyArg, nil);
this.check();
return pyRes;
}
@chpldoc.nodoc
proc newInstance(): PyObjectPtr throws {
var pyRes = PyObject_CallNoArgs(this.get());
this.check();
return pyRes;
}

/*
Create a new instance of a Python class
*/
proc this(const args...): owned Value throws do
return new Value(interpreter, newInstance((...args)), isOwned=true);
@chpldoc.nodoc
proc this(): owned Value throws do
return new Value(interpreter, newInstance(), isOwned=true);

}


Expand Down Expand Up @@ -1681,11 +1706,20 @@ module Python {
:arg idx: The index of the item to get.
:returns: The item at the given index.
*/
pragma "docs only"
proc getItem(type T = owned Value, idx: int): T throws do
compilerError("docs only");

@chpldoc.nodoc
proc getItem(type T, idx: int): T throws {
var item = PySequence_GetItem(this.get(), idx.safeCast(Py_ssize_t));
this.check();
return interpreter.fromPython(T, item);
}
@chpldoc.nodoc
proc getItem(idx: int): owned Value throws do
return this.getItem(owned Value, idx);

/*
Set an item in the list. Equivalent to calling ``obj[idx] = item`` in
Python.
Expand Down Expand Up @@ -2052,6 +2086,10 @@ module Python {
override proc Array.serialize(writer, ref serializer) throws do
writer.write(this:string);

@chpldoc.nodoc
proc NoneType.serialize(writer, ref serializer) throws do
writer.write(this:string);

@chpldoc.nodoc
module CWChar {
require "wchar.h";
Expand Down
23 changes: 15 additions & 8 deletions test/library/packages/Python/correctness/argPassingTest.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ proc test_no_args(mod: borrowed Module) {
var func = new Function(mod, funcName);

func(NoneType);
func();

// error: wrong return type
try { func(int); }
Expand All @@ -23,6 +24,7 @@ proc test_one_arg(mod: borrowed Module) {
var func = new Function(mod, funcName);

func(NoneType, 1);
func(2);

// error: not enough args
try { func(NoneType); }
Expand All @@ -39,8 +41,8 @@ proc test_two_args(mod: borrowed Module) {
var func = new Function(mod, funcName);

func(NoneType, 1, 2);
func(NoneType, "hello", "world");
func(NoneType, None, None);
func("hello", "world");
func(None, None);

// error: not enough args
try { func(NoneType, 3); }
Expand All @@ -58,19 +60,22 @@ proc test_varargs(mod: borrowed Module) {
var func = new Function(mod, funcName);

func(NoneType);
func(NoneType, 1);
func(NoneType, 1, 2);
func(NoneType, 1, 2, 3);
func(NoneType, 1, 2, 3, [1,2,3,]);
func(NoneType, 1, 2, 3, [1,2,3,], 4);
func(1);
func(1, 2);
func(1, 2, 3);
func(1, 2, 3, [1,2,3,]);
func(1, 2, 3, [1,2,3,], 4);
func(NoneType, 1, 2, 3, [1,2,3,], 4, ["key" => "value", "key2" => "value2"]);
func(1, 2, 3, [1,2,3,], 4, ["key" => "value", "key2" => "value2"]);
}
proc test_one_arg_with_default(mod: borrowed Module) {
const funcName = "one_arg_with_default";
var func = new Function(mod, funcName);

func(NoneType);
func(NoneType, 7);
func();
func(7);
}
proc test_three_args_with_default(mod: borrowed Module) {
const funcName = "three_args_with_default";
Expand All @@ -80,6 +85,7 @@ proc test_three_args_with_default(mod: borrowed Module) {
func(NoneType, 8, 9);
func(NoneType, 8, 9, 10);
func(NoneType, 8, kwargs=["c" => 10]);
func(8, kwargs=["c" => 10]);
}
proc test_three_args_with_default_and_kwargs(mod: borrowed Module) {
const funcName = "three_args_with_default_and_kwargs";
Expand All @@ -88,7 +94,7 @@ proc test_three_args_with_default_and_kwargs(mod: borrowed Module) {
func(NoneType, 8);
func(NoneType, 8, 9);
func(NoneType, 8, 9, 10);
func(NoneType, 8, kwargs=["b" => 10]);
func(8, kwargs=["b" => 10]);
func(NoneType, 8, kwargs=["c" => 11, "abc" => 12]);
}
proc test_varargs_and_kwargs(mod: borrowed Module) {
Expand All @@ -99,6 +105,7 @@ proc test_varargs_and_kwargs(mod: borrowed Module) {
func(NoneType, 1);
func(NoneType, 1, ["key" => "value"]);
func(NoneType, kwargs=["a" => 19]);
func(kwargs=["a" => 19]);
func(NoneType, 1, kwargs=["a" => 20]);
func(NoneType, 1, 2, ["key" => "value"], kwargs=["a" => 7, "b" => 8]);
}
Expand Down
10 changes: 10 additions & 0 deletions test/library/packages/Python/correctness/argPassingTest.good
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
called no_args
called no_args
called no_args
Caught PythonException: 'NoneType' object cannot be interpreted as an integer
Caught PythonException: no_args() takes 0 positional arguments but 1 was given
called one_arg with 1
called one_arg with 2
Caught PythonException: one_arg() missing 1 required positional argument: 'a'
Caught PythonException: one_arg() takes 1 positional argument but 2 were given
called two_args with 1 and 2
Expand All @@ -24,12 +26,17 @@ called varargs
args: 1, 2, 3, [1, 2, 3], 4
called varargs
args: 1, 2, 3, [1, 2, 3], 4, {'key2': 'value2', 'key': 'value'}
called varargs
args: 1, 2, 3, [1, 2, 3], 4, {'key2': 'value2', 'key': 'value'}
called one_arg_with_default with 1
called one_arg_with_default with 7
called one_arg_with_default with 1
called one_arg_with_default with 7
called three_args_with_default with 8, 2, and 3
called three_args_with_default with 8, 9, and 3
called three_args_with_default with 8, 9, and 10
called three_args_with_default with 8, 2, and 10
called three_args_with_default with 8, 2, and 10
called three_args_with_default_and_kwargs with 8, 2, and 3
kwargs:
called three_args_with_default_and_kwargs with 8, 9, and 3
Expand All @@ -52,6 +59,9 @@ called varargs_and_kwargs
called varargs_and_kwargs
args:
kwargs: a=19
called varargs_and_kwargs
args:
kwargs: a=19
called varargs_and_kwargs
args: 1
kwargs: a=20
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ proc main() {
var res: [arr.domain] real;
{
var pyRes = new Array(interp, res);
gradModifyArg(NoneType, pyArr, pyRes);
gradModifyArg(pyArr, pyRes);
}
write("gradModifyArg: ", res);
write(" dom: ", res.domain);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ proc testArray(type t, const testArr) {
IO.stdout.flush();
var pyCode = new Module(interp, '__empty__', pythonCode);
var func = new Function(pyCode, 'loop');
func(NoneType, pyArr);
func(pyArr);
IO.stdout.flush();

writeln("from chapel: ", arr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ proc main() {
writeln("lst[2] ", lst.getItem(string, 2));
writeln("lst[lst.size-1] ", lst.getItem(int, lst.size-1));
writeln("lst[-3] ", lst.getItem(owned Value, -3));
writeln("lst[-3] ", lst.getItem(-3));

try {
write("lst[lst.size] ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ lst[1] 2
lst[2] hi
lst[lst.size-1] 5
lst[-3] hi
lst[-3] hi
lst[lst.size] Caught exception: PythonException: list index out of range
lst [100, 2, 'hi', False, 5]
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ proc main() {
var xArr = new Array(interp, x);
var yArr = new Array(interp, y);

var result = saxpy(owned Value, xArr, yArr, a);
var result = saxpy(xArr, yArr, a);
writeln("saxpy on 1D arrays: ", result);
}
writeln();
Expand All @@ -41,7 +41,7 @@ proc main() {
var xArr = new Array(interp, x);
var yArr = new Array(interp, y);

var result = saxpy(owned Value, xArr, yArr, a);
var result = saxpy(xArr, yArr, a);
writeln("saxpy on nested arrays: ", result);
}
writeln();
Expand All @@ -50,7 +50,7 @@ proc main() {
writeln("Before numpyAssign: ", x);
{
var xArr = new Array(interp, x);
numpyAssign(NoneType, xArr);
numpyAssign(xArr);
}
writeln("After numpyAssign: ", x);
}
Expand Down
2 changes: 1 addition & 1 deletion test/library/packages/Python/correctness/basicTypes.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ proc roundTripClass(clsType: borrowed) {
if print then writeln(" obj.getter(): ", res);
assert(res == other);

obj.call(NoneType, "setter", value);
obj.call("setter", value);
res = obj.getAttr(t, "value");
if print then writeln(" obj.value: ", res);
assert(res == value);
Expand Down
Loading

0 comments on commit 6eb636d

Please sign in to comment.