Bug 525860 - [PY4J] Accurate return values
Extended code to try to be more accurate with return values.
Change-Id: Ifd43348f7f406ab05a9f8f449f0df80b753fe80d
Signed-off-by: Martin Kloesch <martin.kloesch@gmail.com>
diff --git a/plugins/org.eclipse.ease.lang.python.py4j/pysrc/ease_py4j_main.py b/plugins/org.eclipse.ease.lang.python.py4j/pysrc/ease_py4j_main.py
index 3094fd0..bb66b2f 100644
--- a/plugins/org.eclipse.ease.lang.python.py4j/pysrc/ease_py4j_main.py
+++ b/plugins/org.eclipse.ease.lang.python.py4j/pysrc/ease_py4j_main.py
@@ -1,4 +1,4 @@
-#################################################################################
+###############################################################################
# Copyright (c) 2016 Kichwa Coders and others.
# All rights reserved. This program and the accompanying materials
# are made available under the terms of the Eclipse Public License v1.0
@@ -7,18 +7,31 @@
#
# Contributors:
# Jonah Graham (Kichwa Coders) - initial API and implementation
-#################################################################################
+###############################################################################
import code
import os
import py4j
from py4j.clientserver import ClientServer, JavaParameters, PythonParameters
-from py4j.java_collections import MapConverter
-from py4j.java_gateway import JavaObject
+from py4j.java_collections import MapConverter, ListConverter, SetConverter
+from py4j.java_gateway import JavaObject, JavaClass
from py4j.protocol import Py4JJavaError, get_command_part
import sys
import threading
import __main__
+import ast
+try:
+ from six import integer_types
+ from six import string_types
+except ImportError:
+ import sys
+ if sys.version_info.major == 2:
+ integer_types = (int, long)
+ string_types = (basestring, )
+ else:
+ integer_types = (int,)
+ string_types = (str,)
+
# To ease some debugging of the py4j engine itself it is useful to turn logging on,
# uncomment the following lines for one way to do that
@@ -26,6 +39,73 @@
# logger = logging.getLogger("py4j")
# logger.setLevel(logging.DEBUG)
# logger.addHandler(logging.StreamHandler())
+def convert_value(value, gw):
+ '''
+ Tries to convert the given value using different strategies.
+
+ Used because default py4j converters have some issues with e.g.
+ nested collections.
+
+ :param value: Value to be converted.
+ :param gw: py4j gateway necessary for converters.
+ '''
+ # None will be mapped to Java null
+ if value is None:
+ return value
+
+ # Integers can be send directly
+ if isinstance(value, integer_types):
+ return value
+
+ # Strings can be send directly
+ if isinstance(value, string_types):
+ return value
+
+ # Recursively check collections
+ if isinstance(value, dict):
+ return MapConverter().convert({
+ convert_value(k, gw): convert_value(v, gw)
+ for k, v in value.items()
+ }, gw)
+ if isinstance(value, list):
+ return ListConverter().convert([
+ convert_value(v, gw) for v in value
+ ], gw)
+ if isinstance(value, set):
+ return SetConverter().convert({
+ convert_value(v, gw) for v in value
+ }, gw)
+
+ # Try registered converters
+ for converter in gw.converters:
+ if converter.can_convert(value):
+ try:
+ return converter.convert(value, gw)
+ except Exception:
+ # TODO: Actually find out what this might throw
+ pass
+
+ # Issues with marshalling Java class objects
+ if isinstance(value, JavaClass):
+ return repr(value)
+
+ # Check if we have a Java object
+ if isinstance(value, py4j.java_gateway.JavaObject):
+ return value
+
+ # Check if we implement a Java interface
+ if hasattr(value, 'Java') and hasattr(getattr(value, 'Java'), 'implements'):
+ if hasattr(value, '_get_obj_id'):
+ if callable(value._get_obj_id):
+ return value
+ else:
+ # Issue with invalid implementations
+ if hasattr(value, 'toString'):
+ return value
+
+ # Last resort use string representation
+ return repr(value)
+
class EaseInteractiveConsole(code.InteractiveConsole):
'''
@@ -35,15 +115,42 @@
def __init__(self, engine, *args, **kwargs):
code.InteractiveConsole.__init__(self, *args, **kwargs)
self.engine = engine
+
def write(self, data):
# Python 3 write the whole error in one write, Python 2 does multiple writes
if self.engine.except_data is None:
self.engine.except_data = [data]
else:
self.engine.except_data.append(data)
+
def runcode(self, code):
try:
- exec(code, self.locals)
+ # If we have compiled code we cannot use AST
+ if isinstance(code, string_types):
+ # Parse code input
+ tree = ast.parse(code)
+
+ # Check if we have multiline statement
+ if len(tree.body) > 1:
+ module = ast.Module(tree.body[:-1])
+ compiled = compile(module, '<...>', 'exec')
+ exec(compiled, self.locals)
+
+ # Check if at least one line given
+ if len(tree.body):
+ if isinstance(tree.body[-1], ast.Expr):
+ # Only expressions can be evaluated
+ expression = ast.Expression(tree.body[-1].value)
+ compiled = compile(expression, '<...>', 'eval')
+ result = eval(compiled, self.locals)
+ return result, False
+ else:
+ module = ast.Module([tree.body[-1]])
+ compiled = compile(module, '<...>', 'exec')
+ exec(compiled, self.locals)
+ else:
+ exec(code, self.locals)
+
except SystemExit:
raise
except Py4JJavaError as e:
@@ -54,52 +161,48 @@
else:
# No java exception here, fallback to normal case
self.showtraceback()
- except:
+ except Exception:
# create information that will end up in a
# ScriptExecutionException
self.showtraceback()
+
+# Sentinel object for no result (different than None)
+NO_RESULT = object()
class InteractiveReturn(object):
'''
Instance of Java's IInteractiveReturn.
This class encapsulates the return state from the
ScriptEngineExecute.executeInteractive() method
'''
- def __init__(self, gateway_client, display_data=None, except_data=None):
+ def __init__(self, gateway_client, display_data=None, except_data=None, result=NO_RESULT):
self.gateway_client = gateway_client
self.display_data = display_data
self.except_data = except_data
+ self.result = result
+
def getException(self):
data = self.except_data
if data is None:
return None
if isinstance(data, JavaObject):
- return data;
- return "".join(data)
- def getResult(self):
- data = self.display_data
-
- try:
- # test if py4j understands this type
- get_command_part(data, dict())
- # py4j knows how to convert this to Java,
- # just return it as is
return data
- except:
- pass
+ return "".join(data)
- # try registered converters
- for converter in self.gateway_client.converters:
- if converter.can_convert(data):
- return converter.convert(data, self.gateway_client)
+ def getResult(self):
+ # Check if we did not receive result
+ if self.result is NO_RESULT:
+ data = self.display_data
+ else:
+ data = self.result
- # data cannot be represented in Java, return a string representation
- # instead
- return repr(data)
+ # Use conversion just to be sure
+ return convert_value(data, self.gateway_client)
class Java:
implements = ['org.eclipse.ease.lang.python.py4j.internal.IInteractiveReturn']
+
class ScriptEngineExecute(object):
'''
Instance of Java's IPythonSideEngine.
@@ -135,7 +238,16 @@
def executeCommon(self, code_text, code_exec):
self.display_data = None
self.except_data = None
- needMore = code_exec(code_text)
+ execution_result = code_exec(code_text)
+
+ # Check if we received a tuple, meaning that we are in script mode
+ if isinstance(execution_result, tuple) and len(execution_result) == 2:
+ result, needMore = execution_result
+ else:
+ # Set result to NO_RESULT to distinguish from None result
+ result = NO_RESULT
+ needMore = execution_result
+
if needMore:
# TODO, need to handle this with prompts, this message
# is a workaround
@@ -145,7 +257,7 @@
except_data = self.except_data
self.display_data = None
self.except_data = None
- return InteractiveReturn(self.gateway._gateway_client, display_data=display_data, except_data=except_data)
+ return InteractiveReturn(self.gateway._gateway_client, display_data=display_data, except_data=except_data, result=result)
def executeScript(self, code_text, filename=None):
# TODO: Handle filename
@@ -155,15 +267,17 @@
return self.executeCommon(code_text, self.interp.push)
def internalGetVariable(self, name):
- return repr(self.locals.get(name))
+ return convert_value(self.locals.get(name), self.gateway._gateway_client)
def internalGetVariables(self):
- locals_repr = dict()
- for k, v in self.locals.items():
- if not k.startswith("__"):
- locals_repr[k] = repr(v)
- converted = MapConverter().convert(locals_repr, self.gateway._gateway_client)
- return converted
+ # Filter out data we do not want
+ filtered = {
+ k: convert_value(v, self.gateway._gateway_client)
+ for k, v in self.locals.items()
+ if not k.startswith('__')
+ }
+
+ return MapConverter().convert(filtered, self.gateway._gateway_client)
def internalHasVariable(self, name):
return name in self.locals
@@ -227,5 +341,6 @@
gateway.shutdown_callback_server()
gateway.shutdown()
+
if __name__ == '__main__':
main(sys.argv)
diff --git a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ModeTestBase.java b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ModeTestBase.java
new file mode 100644
index 0000000..ef1749f
--- /dev/null
+++ b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ModeTestBase.java
@@ -0,0 +1,127 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Kloesch and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Kloesch - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.ease.lang.python.py4j;
+
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.eclipse.ease.IScriptable;
+import org.eclipse.ease.ScriptResult;
+import org.junit.Test;
+
+/**
+ * @author Kloesch
+ *
+ */
+public abstract class ModeTestBase extends Py4JEngineTestBase {
+ abstract protected ScriptResult executeCode(String code) throws Exception;
+
+ abstract protected void executeCode(String code, Object result) throws Exception;
+
+ @Test
+ public void pythonInteger() throws Exception {
+ executeCode("40 + 2", 42);
+ }
+
+ @Test
+ public void pythonString() throws Exception {
+ executeCode("'42'", "42");
+ }
+
+ @Test
+ public void javaInteger() throws Exception {
+ executeCode("java.lang.Integer(42)", 42);
+ }
+
+ @Test
+ public void javaString() throws Exception {
+ executeCode("java.lang.String('42')", "42");
+ }
+
+ @Test
+ public void pythonList() throws Exception {
+ executeCode("[1,2,3]", Arrays.asList(1, 2, 3));
+ }
+
+ @Test
+ public void pythonSet() throws Exception {
+ final Set<Object> javaSet = new HashSet<>();
+ javaSet.add(new Integer(1));
+ javaSet.add(new Integer(2));
+ javaSet.add(new Integer(3));
+ executeCode("{1,2,2,3}", javaSet);
+ }
+
+ @Test
+ public void pythonDict() throws Exception {
+ final Map<String, Object> javaMap = new HashMap<>();
+ javaMap.put("a", "b");
+ executeCode("{'a': 'b'}", javaMap);
+ }
+
+ @Test
+ public void createJavaType() throws Exception {
+ executeCode("java.io.File('/')", new java.io.File("/"));
+ }
+
+ @Test
+ public void createEclipseClass() throws Exception {
+ executeCode("org.eclipse.core.runtime.Path('/')", new org.eclipse.core.runtime.Path("/"));
+ }
+
+ @Test
+ public void createPythonObject() throws Exception {
+ final ScriptResult result = executeCode("object()", false);
+ assertNotNull(result);
+ assertNull(result.getException());
+ assertNotNull(result.getResult());
+ assertTrue(result.getResult() instanceof String);
+ assertThat((String) result.getResult(), startsWith("<object object at "));
+ }
+
+ @Test
+ public void implementJavaInterface() throws Exception {
+ final StringBuilder sb = new StringBuilder();
+ sb.append("class Foo:\n");
+ sb.append("\tdef getSourceCode(self):\n");
+ sb.append("\t\treturn None\n");
+ sb.append("\tdef toString(self):\n");
+ sb.append("\t\treturn java.lang.String(repr(self))\n");
+ sb.append("\tclass Java:\n");
+ sb.append("\t\timplements = ['org.eclipse.ease.IScriptable']\n");
+ executeCode(sb.toString(), false);
+ final ScriptResult result = executeCode("Foo()");
+ assertNotNull(result);
+ assertNull(result.getException());
+ assertNotNull(result.getResult());
+ assertTrue(result.getResult() instanceof IScriptable);
+ }
+
+ @Test
+ public void testMultiple() throws Exception {
+ javaInteger();
+ javaString();
+ createJavaType();
+ createEclipseClass();
+ javaInteger();
+ javaString();
+ }
+}
diff --git a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/Py4JEngineTest.java b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/Py4JEngineTest.java
index 027d139..906707b 100644
--- a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/Py4JEngineTest.java
+++ b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/Py4JEngineTest.java
@@ -24,17 +24,22 @@
push("def a(i):", true);
push(" return i + 1", true);
push("", false);
+
// define b function script mode
assertResultIsNone(executeCode("def b(i):\n\treturn i + 2", false));
// try to use each function from the other mode
- assertEquals("11", printExpression("a(10)"));
- assertEquals("12", printExpression("b(10)"));
+ final ScriptResult resultScript_a = executeCode("a(20)", false);
+ assertNull(resultScript_a.getException());
+ assertEquals(21, resultScript_a.getResult());
+ final ScriptResult resultScript_b = executeCode("b(10)", false);
+ assertNull(resultScript_b.getException());
+ assertEquals(12, resultScript_b.getResult());
- ScriptResult result21 = executeCode("a(20)", true);
+ final ScriptResult result21 = executeCode("a(20)", true);
assertNull(result21.getException());
assertEquals(21, result21.getResult());
- ScriptResult result22 = executeCode("b(20)", true);
+ final ScriptResult result22 = executeCode("b(20)", true);
assertNull(result22.getException());
assertEquals(22, result22.getResult());
}
diff --git a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ScriptModeEngineTest.java b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ScriptModeEngineTest.java
index f3ad338..b32a213 100644
--- a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ScriptModeEngineTest.java
+++ b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ScriptModeEngineTest.java
@@ -13,8 +13,8 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.CoreMatchers.startsWith;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
@@ -23,61 +23,26 @@
import org.junit.Ignore;
import org.junit.Test;
-public class ScriptModeEngineTest extends Py4JEngineTestBase {
+public class ScriptModeEngineTest extends ModeTestBase {
+ @Override
protected ScriptResult executeCode(String code) throws Exception {
return super.executeCode(code, false);
}
- @Test
- public void pythonInteger() throws Exception {
- assertEquals("42", printExpression("40 + 2"));
- }
-
- @Test
- public void pythonString() throws Exception {
- assertEquals("42", printExpression("'42'"));
- }
-
- @Test
- public void javaInteger() throws Exception {
- assertEquals("42", printExpression("java.lang.Integer(42)"));
- }
-
- @Test
- public void javaString() throws Exception {
- assertEquals("42", printExpression("java.lang.String('42')"));
- }
-
- @Test
- public void createJavaType() throws Exception {
- assertEquals(new java.io.File("/").toString(), printExpression("java.io.File('/')"));
- }
-
- @Test
- public void createEclipseClass() throws Exception {
- assertEquals(new org.eclipse.core.runtime.Path("/").toString(), printExpression("org.eclipse.core.runtime.Path('/')"));
- }
-
- @Test
- public void createPythonObject() throws Exception {
- assertThat(printExpression("object()"), startsWith("<object object at "));
- }
-
- @Test
- public void testMultiple() throws Exception {
- javaInteger();
- javaString();
- createJavaType();
- createEclipseClass();
- javaInteger();
- javaString();
+ @Override
+ protected void executeCode(String code, Object target) throws Exception {
+ final ScriptResult result = super.executeCode(code, false);
+ assertNotNull(result);
+ assertNull(result.getException());
+ assertNotNull(result.getResult());
+ assertEquals(target, result.getResult());
}
@Test
@Ignore("Disabled until Bug 493677 is resolved")
public void callExit() throws Exception {
- ScriptResult result = executeCode("print_('this should be output', False)\nexit()\nprint_('this should not appear')");
+ final ScriptResult result = executeCode("print_('this should be output', False)\nexit()\nprint_('this should not appear')");
assertResultIsNone(result);
assertEquals("this should be output", fOutputStream.getAndClearOutput());
}
@@ -89,7 +54,7 @@
@Test
public void incompleteStatement() throws Exception {
- ScriptResult result = executeCode("def a():");
+ final ScriptResult result = executeCode("def a():");
assertThat(result.getException(), instanceOf(ScriptExecutionException.class));
assertThat(fErrorStream.getAndClearOutput(), containsString("SyntaxError"));
assertNull(result.getResult());
@@ -109,8 +74,7 @@
@Test
public void multiLineStatement() throws Exception {
- assertResultIsNone(executeCode("def a():\n\treturn 42"));
- assertEquals("42", printExpression("a()"));
+ executeCode("def a():\n\treturn 42\na()", 42);
}
@Test
diff --git a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ShellModeEngineTest.java b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ShellModeEngineTest.java
index 5d6c0e6..91d078f 100644
--- a/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ShellModeEngineTest.java
+++ b/tests/org.eclipse.ease.lang.python.py4j.test/src/org/eclipse/ease/lang/python/py4j/ShellModeEngineTest.java
@@ -13,112 +13,36 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.CoreMatchers.startsWith;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-
import org.eclipse.ease.ScriptExecutionException;
import org.eclipse.ease.ScriptResult;
import org.junit.Ignore;
import org.junit.Test;
-public class ShellModeEngineTest extends Py4JEngineTestBase {
+public class ShellModeEngineTest extends ModeTestBase {
+ @Override
protected ScriptResult executeCode(String code) throws Exception {
return super.executeCode(code, true);
}
- @Test
- public void simpleScriptCode() throws Exception {
- ScriptResult result = executeCode("40 + 2");
+ @Override
+ protected void executeCode(String code, Object target) throws Exception {
+ final ScriptResult result = super.executeCode(code, true);
+ assertNotNull(result);
assertNull(result.getException());
- assertEquals(42, result.getResult());
- }
-
- @Test
- public void createAutoConvertedJavaInteger() throws Exception {
- ScriptResult result = executeCode("java.lang.Integer(42)");
- assertNull(result.getException());
- assertEquals(42, result.getResult());
- }
-
- @Test
- public void createAutoConvertedJavaString() throws Exception {
- ScriptResult result = executeCode("java.lang.String('42')");
- assertNull(result.getException());
- assertEquals("42", result.getResult());
- }
-
- @Test
- public void createList() throws Exception {
- ScriptResult result = executeCode("[1, 2, 3]");
- assertNull(result.getException());
- assertEquals(Arrays.asList(1, 2, 3), result.getResult());
- }
-
- @Test
- public void createMap() throws Exception {
- ScriptResult result = executeCode("dict(a=1, b=2, c=3)");
- assertNull(result.getException());
- Map<Object, Object> map = new HashMap<>();
- map.put("a", 1);
- map.put("b", 2);
- map.put("c", 3);
- assertEquals(map, result.getResult());
- }
-
- @Test
- public void createSet() throws Exception {
- ScriptResult result = executeCode("set([1, 2, 3])");
- assertNull(result.getException());
- Set<Object> set = new HashSet<>();
- set.add(1);
- set.add(2);
- set.add(3);
- assertEquals(set, result.getResult());
- }
-
- @Test
- public void createJavaType() throws Exception {
- ScriptResult result = executeCode("java.io.File('/')");
- assertNull(result.getException());
- assertEquals(new java.io.File("/"), result.getResult());
- }
-
- @Test
- public void createEclipseClass() throws Exception {
- ScriptResult result = executeCode("org.eclipse.core.runtime.Path('/')");
- assertNull(result.getException());
- assertEquals(new org.eclipse.core.runtime.Path("/"), result.getResult());
- }
-
- @Test
- public void createPythonObject() throws Exception {
- ScriptResult result = executeCode("object()");
- assertNull(result.getException());
- assertThat(result.getResult(), instanceOf(String.class));
- assertThat((String) result.getResult(), startsWith("<object object at "));
- }
-
- @Test
- public void testMultiple() throws Exception {
- simpleScriptCode();
- createJavaType();
- createEclipseClass();
- simpleScriptCode();
+ assertNotNull(result.getResult());
+ assertEquals(target, result.getResult());
}
@Test
public void callModuleCode() throws Exception {
- ScriptResult result = executeCode("exit(\"done\")");
+ final ScriptResult result = executeCode("exit(\"done\")");
assertNull(result.getException());
assertEquals("done", result.getResult());
}
@@ -130,7 +54,7 @@
@Test
public void getScriptEngine() throws Exception {
- ScriptResult result = executeCode("getScriptEngine()");
+ final ScriptResult result = executeCode("getScriptEngine()");
assertNull(result.getException());
assertSame(fEngine, result.getResult());
}
@@ -172,15 +96,16 @@
public void multiLineStatement() throws Exception {
push("def a():", true);
push(" return 42", true);
- assertResultIsNone(push("", false));
- ScriptResult result = push("a()", false);
+ push("", false);
+ // assertResultIsNone(push("", false));final
+ final ScriptResult result = push("a()", false);
assertNull(result.getException());
assertEquals(42, result.getResult());
}
@Test
public void invalidSyntax() throws Exception {
- ScriptResult result = executeCode("1++");
+ final ScriptResult result = executeCode("1++");
assertThat(result.getException(), instanceOf(ScriptExecutionException.class));
assertThat(fErrorStream.getAndClearOutput(), containsString("SyntaxError"));
assertNull(result.getResult());
@@ -188,7 +113,7 @@
@Test
public void runtimeError() throws Exception {
- ScriptResult result = executeCode("x");
+ final ScriptResult result = executeCode("x");
assertThat(result.getException(), instanceOf(ScriptExecutionException.class));
assertThat(fErrorStream.getAndClearOutput(), containsString("NameError"));
assertNull(result.getResult());
