Skip to content
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

Function for converting Python values to an explicit Java type #128

Merged
merged 30 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9bdbaeb
Add method to convert Python value to a Java object of the specified …
rbasralian Sep 23, 2022
06df869
make some return values more clear/meaningful
rbasralian Jan 6, 2023
d599bf0
update some tests to match current behavior
rbasralian Jan 6, 2023
1676e46
Fix bug when casting using a type name (not a JPy_JType). Add method …
rbasralian Jan 9, 2024
d43e8df
Make as_jobj actually work. Clarify variable names in cast_internal a…
rbasralian Jan 9, 2024
20444d6
Update .gitignore
rbasralian Jan 9, 2024
5a3fdf0
Remove a stray print()
rbasralian Jan 9, 2024
ea60b1f
dev version bump
rbasralian Jan 9, 2024
61bde34
update types in test
rbasralian Jan 10, 2024
877a99b
comments on failing test
rbasralian Jan 10, 2024
36ed7a0
more logging for tests
rbasralian Jan 10, 2024
b49090b
Merge remote-tracking branch 'origin/master' into raffi_obj_conv
rbasralian Jan 10, 2024
3dea91c
Merge remote-tracking branch 'origin/master' into raffi_obj_conv
rbasralian Jan 11, 2024
0996fac
Fix potential memory leak
rbasralian Jan 11, 2024
2794f6b
Replace get_type_name() with `.jclassname` attribute
rbasralian Jan 11, 2024
e20757b
Clean up redundant conditions/extra parentheses
rbasralian Jan 12, 2024
525362d
more tests
rbasralian Jan 12, 2024
2d252c3
Replace deprecated assertion
rbasralian Jan 12, 2024
c25ab7c
formatting
rbasralian Jan 12, 2024
eaff9d6
Tests for converting to primitive and boxed arrays
rbasralian Jan 12, 2024
dc161cf
Update test for explicitly converting to PyObject. Add more details o…
rbasralian Jan 12, 2024
d204645
Update deprecated assertion
rbasralian Jan 12, 2024
1d884a8
fix broken test
rbasralian Jan 17, 2024
c61869d
add a macro for building Python Nones (separate from`FROM_JVOID` and …
rbasralian Jan 17, 2024
010feaf
comment out nonfunctional test
rbasralian Jan 17, 2024
b060d17
update test to support newer JVMs
rbasralian Jan 17, 2024
bdd1f72
Move convert-to-PyObject test to another file (and run it with a clas…
rbasralian Jan 17, 2024
efab329
consistent capitalization in print()s
rbasralian Jan 17, 2024
4182956
Fix incorrect returns of `Py_NONE`
rbasralian Jan 17, 2024
f364d46
Rename `as_jobj` to `convert`
rbasralian Jan 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ Vagrantfile
*.so
*.dll

.vscode/
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ documentation.
4. Update documentation, `cd doc` and run `make html`
5. http://peterdowns.com/posts/first-time-with-pypi.html


Running Tests
----------------

Run: `python setup.py build test`

Automated builds
----------------

Expand Down
5 changes: 4 additions & 1 deletion doc/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ jpy Functions
tm = rt.totalMemory()

The returned Java types have a `jclass` attribute which returns the actual Java object. This allows for using
the Java types where a Java method would expect a parameter of type `java.lang.Class`.
the Java types where a Java method would expect a parameter of type `java.lang.Class`. They also have a `jclassname`
attribute, which returns the Java type associated with the 'obj' reference. The `jclassname` is the reference's
declared type (which may be a supertype), rather than the object's runtime type (as returned by
`obj.getClass().getName()`).

To instantiate Java array objects, the :py:func:`jpy.array()` function is used.

Expand Down
11 changes: 6 additions & 5 deletions src/main/c/jpy_jtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -914,19 +914,19 @@ int JType_ConvertPythonToJavaObject(JNIEnv* jenv, JPy_JType* type, PyObject* pyA
return JType_CreateJavaDoubleObject(jenv, type, pyArg, objectRef);
} else if (type == JPy_JPyObject) {
return JType_CreateJavaPyObject(jenv, type, pyArg, objectRef);
} else if (JPy_IS_STR(pyArg) && (type == JPy_JString || type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_JString->classRef, type->classRef)))) {
} else if (JPy_IS_STR(pyArg) && (type == JPy_JString || type == JPy_JObject || (*jenv)->IsAssignableFrom(jenv, JPy_JString->classRef, type->classRef))) {
return JPy_AsJString(jenv, pyArg, objectRef);
} else if (PyBool_Check(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Boolean_JClass, type->classRef)))) {
} else if (PyBool_Check(pyArg) && (type == JPy_JObject || (*jenv)->IsAssignableFrom(jenv, JPy_Boolean_JClass, type->classRef))) {
return JType_CreateJavaBooleanObject(jenv, type, pyArg, objectRef);
} else if (JPy_IS_CLONG(pyArg) && (type == JPy_JObject || (*jenv)->IsAssignableFrom(jenv, JPy_Number_JClass, type->classRef))) {
return JType_CreateJavaNumberFromPythonInt(jenv, type, pyArg, objectRef);
} else if (JPy_IS_CLONG(pyArg) && ((*jenv)->IsAssignableFrom(jenv, JPy_Integer_JClass, type->classRef))) {
return JType_CreateJavaIntegerObject(jenv, type, pyArg, objectRef);
} else if (JPy_IS_CLONG(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Long_JClass, type->classRef)))) {
} else if (JPy_IS_CLONG(pyArg) && (*jenv)->IsAssignableFrom(jenv, JPy_Long_JClass, type->classRef)) {
return JType_CreateJavaLongObject(jenv, type, pyArg, objectRef);
} else if (PyFloat_Check(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Double_JClass, type->classRef)))) {
} else if (PyFloat_Check(pyArg) && (type == JPy_JObject || (*jenv)->IsAssignableFrom(jenv, JPy_Double_JClass, type->classRef))) {
return JType_CreateJavaDoubleObject(jenv, type, pyArg, objectRef);
} else if (PyFloat_Check(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Float_JClass, type->classRef)))) {
} else if (PyFloat_Check(pyArg) && (*jenv)->IsAssignableFrom(jenv, JPy_Float_JClass, type->classRef)) {
Comment on lines +925 to +929
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on the effect of this? It looks like you removed the JPy_JObject condition from Long / Float, but not Double?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cases I changed were:

    } else if (JPy_IS_CLONG(pyArg) && (type == JPy_JObject || (*jenv)->IsAssignableFrom(jenv, JPy_Number_JClass, type->classRef))) {
        return JType_CreateJavaNumberFromPythonInt(jenv, type, pyArg, objectRef);
    } else if (JPy_IS_CLONG(pyArg) && ((*jenv)->IsAssignableFrom(jenv, JPy_Integer_JClass, type->classRef))) {
        return JType_CreateJavaIntegerObject(jenv, type, pyArg, objectRef);
    } else if (JPy_IS_CLONG(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Long_JClass, type->classRef)))) {
        return JType_CreateJavaLongObject(jenv, type, pyArg, objectRef);

and:

    } else if (PyFloat_Check(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Double_JClass, type->classRef)))) {
        return JType_CreateJavaDoubleObject(jenv, type, pyArg, objectRef);
    } else if (PyFloat_Check(pyArg) && (type == JPy_JObject || ((*jenv)->IsAssignableFrom(jenv, JPy_Float_JClass, type->classRef)))) {
        return JType_CreateJavaFloatObject(jenv, type, pyArg, objectRef);
    }

in both case, the second type == JPy_JObject test will always be false — for the first case, we will have already landed in JType_CreateJavaNumberFromPythonInt and in the second case we'd hit JType_CreateJavaDoubleObject.

return JType_CreateJavaFloatObject(jenv, type, pyArg, objectRef);
} else if (type == JPy_JObject && allowObjectWrapping) {
return JType_CreateJavaPyObject(jenv, JPy_JPyObject, pyArg, objectRef);
Expand Down Expand Up @@ -1298,6 +1298,7 @@ int JType_AddClassAttribute(JNIEnv* jenv, JPy_JType* declaringClass)
return -1;
}
PyDict_SetItem(typeDict, Py_BuildValue("s", "jclass"), (PyObject*) JObj_FromType(jenv, JPy_JClass, declaringClass->classRef));
PyDict_SetItem(typeDict, Py_BuildValue("s", "jclassname"), (PyObject*) JPy_FROM_CSTR(declaringClass->typeObj.tp_name));
}
return 0;
}
Expand Down
109 changes: 93 additions & 16 deletions src/main/c/jpy_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ PyObject* JPy_create_jvm(PyObject* self, PyObject* args, PyObject* kwds);
PyObject* JPy_destroy_jvm(PyObject* self, PyObject* args);
PyObject* JPy_get_type(PyObject* self, PyObject* args, PyObject* kwds);
PyObject* JPy_cast(PyObject* self, PyObject* args);
PyObject* JPy_as_jobj(PyObject* self, PyObject* args);
PyObject* JPy_array(PyObject* self, PyObject* args);
PyObject* JPy_byte_buffer(PyObject* self, PyObject* args);

Expand All @@ -61,6 +62,11 @@ static PyMethodDef JPy_Functions[] = {
"cast(obj, type) - Cast the given Java object to the given Java type (type name or type object). "
"Returns None if the cast is not possible."},

{"as_jobj", JPy_as_jobj, METH_VARARGS,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool - previously, we could do this, it just required explicit java code:

public class MyClass {
  public static MyJavaType identity(MyJavaType x) {
    return x;
  }
}
jobj = jpy.get_type('MyClass').identity(pobj)

I'm assuming these are the semantics you are after?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, basically I had two parallel lists, one of java types and one of python values (in practice strings/numbers), and I wanted to stick the python values into a java Object[] with the given Java type, but numbers I wanted as Long were getting converted to Byte. I could have just used the constructors explicitly but that's not as clean, and it seems worthwhile to expose JType_ConvertPythonToJavaObject anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not too crazy about the name as_jobj, would conv be better?
From what I can gather, I do like that I can now create a org.jpy.PyObject in Python directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely open to changing it. I went with as_jobj because it ultimately calls JPy_AsJObjectWithType, and I thought having 'obj' in the name explicitly could be helpful because conversion in Java is a distinct concept (though a related one) that also involves Java primitives.

@devinrsmith what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "as" gives off the wrong semantic notion, somewhat akin to zero-cost. While the same could be said about JPy_AsJObjectWithType, that's lower level and not exposed to the end user. Also, it calls down into JType_ConvertPythonToJavaObject.

I'm also not the biggest fan of trying to save a few characters; my vote would be for jpy.convert instead of jpy.conv / jpy.as_jobj.

"as_jobj(obj, type) - Convert the given Python object to the given Java type (type name or type object). "
"Returns None if the conversion is not possible. If the Java type is a primitive, the returned object "
"will be of the corresponding boxed type."},

{"array", JPy_array, METH_VARARGS,
"array(name, init) - Return a new Java array of given Java type (type name or type object) and initializer (array length or sequence). "
"Possible primitive types are 'boolean', 'byte', 'char', 'short', 'int', 'long', 'float', and 'double'."},
Expand Down Expand Up @@ -451,7 +457,7 @@ PyObject* JPy_create_jvm(PyObject* self, PyObject* args, PyObject* kwds)
if (JPy_JVM != NULL) {
JPy_DIAG_PRINT(JPy_DIAG_F_JVM + JPy_DIAG_F_ERR, "JPy_create_jvm: WARNING: Java VM is already running.\n");
JPy_DECREF(options);
return Py_BuildValue("");
return JPy_FROM_JVOID();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JPy_FROM_JVOID is meant to be used in combination with void.class type; it's not appropriate to use it in these contexts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the handful of Py_BuildValue()s I changed with a new JPy_PY_NONE()

}

if (!PySequence_Check(options)) {
Expand Down Expand Up @@ -513,7 +519,7 @@ PyObject* JPy_create_jvm(PyObject* self, PyObject* args, PyObject* kwds)
return NULL;
}

return Py_BuildValue("");
return JPy_FROM_JVOID();
}

PyObject* JPy_destroy_jvm(PyObject* self, PyObject* args)
Expand All @@ -526,7 +532,7 @@ PyObject* JPy_destroy_jvm(PyObject* self, PyObject* args)
JPy_JVM = NULL;
}

return Py_BuildValue("");
return JPy_FROM_JVOID();
}

PyObject* JPy_get_type_internal(JNIEnv* jenv, PyObject* self, PyObject* args, PyObject* kwds)
Expand All @@ -551,41 +557,41 @@ PyObject* JPy_get_type(PyObject* self, PyObject* args, PyObject* kwds)
PyObject* JPy_cast_internal(JNIEnv* jenv, PyObject* self, PyObject* args)
{
PyObject* obj;
PyObject* objType;
JPy_JType* type;
PyObject* targetTypeArg; // can be a string PyObject (i.e. JPy_IS_STR) or a JPy_JType
JPy_JType* targetTypeParsed; // the actual type that targetTypeArg is processed as
jboolean inst;

if (!PyArg_ParseTuple(args, "OO:cast", &obj, &objType)) {
if (!PyArg_ParseTuple(args, "OO:cast", &obj, &targetTypeArg)) {
return NULL;
}

if (obj == Py_None) {
return Py_BuildValue("");
return JPy_FROM_JNULL();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not appropriate to use here unless we are converting from a null java object.

}

if (!JObj_Check(obj)) {
PyErr_SetString(PyExc_ValueError, "cast: argument 1 (obj) must be a Java object");
return NULL;
}

if (JPy_IS_STR(objType)) {
const char* typeName = JPy_AS_UTF8(objType);
type = JType_GetTypeForName(jenv, typeName, JNI_FALSE);
if (type == NULL) {
if (JPy_IS_STR(targetTypeArg)) {
const char* typeName = JPy_AS_UTF8(targetTypeArg);
targetTypeParsed = JType_GetTypeForName(jenv, typeName, JNI_FALSE);
if (targetTypeParsed == NULL) {
return NULL;
}
} else if (JType_Check(objType)) {
type = (JPy_JType*) objType;
} else if (JType_Check(targetTypeArg)) {
targetTypeParsed = (JPy_JType*) targetTypeArg;
} else {
PyErr_SetString(PyExc_ValueError, "cast: argument 2 (obj_type) must be a Java type name or Java type object");
return NULL;
}

inst = (*jenv)->IsInstanceOf(jenv, ((JPy_JObj*) obj)->objectRef, type->classRef);
inst = (*jenv)->IsInstanceOf(jenv, ((JPy_JObj*) obj)->objectRef, targetTypeParsed->classRef);
if (inst) {
return (PyObject*) JObj_FromType(jenv, (JPy_JType*) objType, ((JPy_JObj*) obj)->objectRef);
return (PyObject*) JObj_FromType(jenv, (JPy_JType*) targetTypeParsed, ((JPy_JObj*) obj)->objectRef);
} else {
return Py_BuildValue("");
return JPy_FROM_JNULL();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

}
}

Expand All @@ -594,6 +600,77 @@ PyObject* JPy_cast(PyObject* self, PyObject* args)
JPy_FRAME(PyObject*, NULL, JPy_cast_internal(jenv, self, args), 16)
}

PyObject* JPy_as_jobj_internal(JNIEnv* jenv, PyObject* self, PyObject* args)
{
PyObject* obj;
PyObject* targetTypeArg; // can be a string PyObject (i.e. JPy_IS_STR) or a JPy_JType
JPy_JType* targetTypeParsed; // the actual type that targetTypeArg is processed as
jboolean inst;

PyObject* resultObj;
jobject objectRef;

// Parse the 'args' from Python into 'obj'/'objType'.
if (!PyArg_ParseTuple(args, "OO:as_jobj", &obj, &targetTypeArg)) {
return NULL;
}

if (obj == Py_None) {
return JPy_FROM_JNULL();
}

if (JPy_IS_STR(targetTypeArg)) {
const char* typeName = JPy_AS_UTF8(targetTypeArg);
targetTypeParsed = JType_GetTypeForName(jenv, typeName, JNI_FALSE);
if (targetTypeParsed == NULL) {
return NULL;
}
} else if (JType_Check(targetTypeArg)) {
targetTypeParsed = (JPy_JType*) targetTypeArg;
} else {
PyErr_SetString(PyExc_ValueError, "cast: argument 2 (obj_type) must be a Java type name or Java type object");
return NULL;
}

// If the input obj is a Java object, and it is already of the specified target type,
// then just cast it.
if (JObj_Check(obj)) {
inst = (*jenv)->IsInstanceOf(jenv, ((JPy_JObj*) obj)->objectRef, targetTypeParsed->classRef);
if (inst) {
return (PyObject*) JObj_FromType(jenv, (JPy_JType*) targetTypeParsed, ((JPy_JObj*) obj)->objectRef);
}
}

// Convert the Python object to the specified Java type
if (JPy_AsJObjectWithType(jenv, obj, &objectRef, targetTypeParsed) < 0) {
return NULL;
}

// Create a global reference for the objectRef (so it is valid after we exit this frame)
objectRef = (*jenv)->NewGlobalRef(jenv, objectRef);
if (objectRef == NULL) {
PyErr_NoMemory();
return NULL;
}

// Create a PyObject (JObj) to hold the result
resultObj = (JPy_JObj*) PyObject_New(JPy_JObj, JTYPE_AS_PYTYPE(targetTypeParsed));
if (resultObj == NULL) {
(*jenv)->DeleteGlobalRef(jenv, objectRef);
return NULL;
}
// Store the reference to the converted object in the result JObj
((JPy_JObj*) resultObj)->objectRef = objectRef;

return (PyObject*) resultObj;
}


PyObject* JPy_as_jobj(PyObject* self, PyObject* args)
{
JPy_FRAME(PyObject*, NULL, JPy_as_jobj_internal(jenv, self, args), 16)
}

PyObject* JPy_array_internal(JNIEnv* jenv, PyObject* self, PyObject* args)
{
JPy_JType* componentType;
Expand Down
6 changes: 6 additions & 0 deletions src/test/java/org/jpy/EmbeddableTestJunit.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
package org.jpy;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;

/**
* Wraps up {@link EmbeddableTest} with JUnit so {@link EmbeddableTest} doesn't have to have the
* JUnit dependency.
*/
public class EmbeddableTestJunit {

@Rule
public TestRule testStatePrinter = new TestStatePrinter();

@Test
public void testStartingAndStoppingIfAvailable() {
EmbeddableTest.testStartingAndStoppingIfAvailable();
Expand Down
5 changes: 5 additions & 0 deletions src/test/java/org/jpy/JavaReflectionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import org.jpy.annotations.Mutable;
import org.jpy.annotations.Return;
import org.jpy.fixtures.MethodReturnValueTestFixture;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;

import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
Expand All @@ -34,6 +36,9 @@
*/
public class JavaReflectionTest {

@Rule
public TestRule testStatePrinter = new TestStatePrinter();

@Test
public void testPrimitiveAndVoidNames() throws Exception {
assertEquals("boolean", Boolean.TYPE.getName());
Expand Down
6 changes: 6 additions & 0 deletions src/test/java/org/jpy/LifeCycleTest.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package org.jpy;

import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;

public class LifeCycleTest {

@Rule
public TestRule testStatePrinter = new TestStatePrinter();

private static final boolean ON_WINDOWS = System.getProperty("os.name").toLowerCase().contains("windows");

@Test
Expand Down
4 changes: 4 additions & 0 deletions src/test/java/org/jpy/PyLibTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,14 @@
import java.util.Map;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestRule;

public class PyLibTest {

@Rule
public TestRule testStatePrinter = new TestStatePrinter();
@Before
public void setUp() throws Exception {
//PyLib.Diag.setFlags(PyLib.Diag.F_ERR);
Expand Down
4 changes: 4 additions & 0 deletions src/test/java/org/jpy/PyLibWithSysPathTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.jpy;

import org.junit.*;
import org.junit.rules.TestRule;

import java.io.File;
import java.net.URI;
Expand All @@ -27,6 +28,9 @@

public class PyLibWithSysPathTest {

@Rule
public TestRule testStatePrinter = new TestStatePrinter();

@Before
public void setUp() throws Exception {

Expand Down
3 changes: 3 additions & 0 deletions src/test/java/org/jpy/PyModuleTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.jpy;

import org.junit.*;
import org.junit.rules.TestRule;

import java.io.File;

Expand All @@ -29,6 +30,8 @@
*/
public class PyModuleTest {

@Rule
public TestRule testStatePrinter = new TestStatePrinter();
@Before
public void setUp() throws Exception {
//System.out.println("PyModuleTest: Current thread: " + Thread.currentThread());
Expand Down
Loading
Loading