Repository: OpenRefine/OpenRefine (java)

Issue:
Jython should support date passing
Currently the Jython integration knows how to translate strings, ints, longs, and floats.  It should be taught to handle one or more formats that interoperate with Refine's datetime type.



Pull request:
Add date handling to Jython.  Fixes #692
Fixes #692 

Changes proposed in this pull request:
- adds support for passing dates into and out of Jython
- includes tests for passing dates both directions
- simplifies sequence unwrapping
- add tests for sequences, ints, floats, strings


Patch:
diff --git a/.gitattributes b/.gitattributes
index 7057ad030..412eeda78 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,5 +1,5 @@
 # Auto detect text files and perform LF normalization
-* text=off
+* text=auto
 
 # Custom for Visual Studio
 *.cs     diff=csharp
diff --git a/extensions/jython/src/com/google/refine/jython/JythonEvaluable.java b/extensions/jython/src/com/google/refine/jython/JythonEvaluable.java
index e240320f4..bcccb4028 100644
--- a/extensions/jython/src/com/google/refine/jython/JythonEvaluable.java
+++ b/extensions/jython/src/com/google/refine/jython/JythonEvaluable.java
@@ -34,8 +34,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 package com.google.refine.jython;
 
 import java.io.File;
+import java.sql.Timestamp;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Properties;
 
@@ -128,10 +130,19 @@ public class JythonEvaluable implements Evaluable {
     @Override
     public Object evaluate(Properties bindings) {
         try {
+            Object value = bindings.get("value");
+            PyObject pyValue;
+            if (value instanceof OffsetDateTime) {
+                // We know the OffsetDateTime is always at UTC, but just in case it ever changes, do the UTC change
+                pyValue = Py.newDatetime(Timestamp.valueOf(((OffsetDateTime) value).atZoneSameInstant(ZoneOffset.UTC).toLocalDateTime()));
+            } else {
+                pyValue = Py.java2py(value);
+            }
             // call the temporary PyFunction directly
             Object result = ((PyFunction) _engine.get(s_functionName)).__call__(
+
                     new PyObject[] {
-                            Py.java2py(bindings.get("value")),
+                            pyValue,
                             new JythonHasFieldsWrapper((HasFields) bindings.get("cell"), bindings),
                             new JythonHasFieldsWrapper((HasFields) bindings.get("cells"), bindings),
                             new JythonHasFieldsWrapper((HasFields) bindings.get("row"), bindings),
@@ -169,16 +180,23 @@ public class JythonEvaluable implements Evaluable {
     protected Object unwrap(PyObject po) {
         if (po instanceof PyNone) {
             return null;
+        } else if ("datetime".equals(po.getType().getName())) {
+            return OffsetDateTime.of(
+                    po.__getattr__("year").asInt(),
+                    po.__getattr__("month").asInt(),
+                    po.__getattr__("day").asInt(),
+                    po.__getattr__("hour").asInt(),
+                    po.__getattr__("minute").asInt(),
+                    po.__getattr__("second").asInt(),
+                    po.__getattr__("microsecond").asInt() * 1000, // scale to nanoseconds
+                    ZoneOffset.UTC);
         } else if (po.isNumberType()) {
             return po.asDouble();
         } else if (po.isSequenceType()) {
-            Iterator<PyObject> i = po.asIterable().iterator();
-
-            List<Object> list = new ArrayList<Object>();
-            while (i.hasNext()) {
-                list.add(unwrap((Object) i.next()));
+            List<Object> list = new ArrayList<>();
+            for (Object o : po.asIterable()) {
+                list.add(unwrap(o));
             }
-
             return list.toArray();
         } else {
             return po;


Test patch:
diff --git a/extensions/jython/tests/src/com/google/refine/jython/JythonEvaluableTest.java b/extensions/jython/tests/src/com/google/refine/jython/JythonEvaluableTest.java
index f97903336..733a232cd 100644
--- a/extensions/jython/tests/src/com/google/refine/jython/JythonEvaluableTest.java
+++ b/extensions/jython/tests/src/com/google/refine/jython/JythonEvaluableTest.java
@@ -1,6 +1,8 @@
 
 package com.google.refine.jython;
 
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
 import java.util.Properties;
 
 import org.python.core.PyObject;
@@ -18,26 +20,29 @@ import com.google.refine.model.Row;
  */
 public class JythonEvaluableTest {
 
-    // Reproduces the situation when result is a PyObject
-    // Version with a test case which only calls the existing evaluate method
-    @Test
-    public void unwrapPyObjectTest() {
-        Properties props = new Properties();
+    private Properties createBindings() {
         Project project = new Project();
-
         Row row = new Row(2);
         row.setCell(0, new Cell("one", null));
         row.setCell(0, new Cell("1", null));
+        Properties bindings = new Properties();
+        bindings.put("columnName", "number");
+        bindings.put("true", "true");
+        bindings.put("false", "false");
+        bindings.put("rowIndex", "0");
+        bindings.put("value", 1);
+        bindings.put("project", project);
+        bindings.put("call", "number");
+        bindings.put("PI", "3.141592654");
+        bindings.put("cells", new CellTuple(project, row));
+        return bindings;
+    }
 
-        props.put("columnName", "number");
-        props.put("true", "true");
-        props.put("false", "false");
-        props.put("rowIndex", "0");
-        props.put("value", 1);
-        props.put("project", project);
-        props.put("call", "number");
-        props.put("PI", "3.141592654");
-        props.put("cells", new CellTuple(project, row));
+    // Reproduces the situation when result is a PyObject
+    // Version with a test case which only calls the existing evaluate method
+    @Test
+    public void unwrapPyObjectTest() {
+        Properties props = createBindings();
         String funcExpression = "class Foo(object):\n" +
                 "    bar = 1\n" +
                 "\n" +
@@ -49,22 +54,7 @@ public class JythonEvaluableTest {
 
     @Test
     public void testJythonConcurrent() {
-        Properties props = new Properties();
-        Project project = new Project();
-
-        Row row = new Row(2);
-        row.setCell(0, new Cell("one", null));
-        row.setCell(0, new Cell("1", null));
-
-        props.put("columnName", "number");
-        props.put("true", "true");
-        props.put("false", "false");
-        props.put("rowIndex", "0");
-        props.put("value", 1);
-        props.put("project", project);
-        props.put("call", "number");
-        props.put("PI", "3.141592654");
-        props.put("cells", new CellTuple(project, row));
+        Properties props = createBindings();
 
         Evaluable eval1 = new JythonEvaluable("a = value\nreturn a * 2");
         Long value1 = (Long) eval1.evaluate(props);
@@ -76,4 +66,71 @@ public class JythonEvaluableTest {
         Long value2 = (Long) eval1.evaluate(props);
         Assert.assertEquals(value1, value2);
     }
+
+    @Test
+    public void testJythonDate() {
+        Properties bindings = createBindings();
+        OffsetDateTime expectedDate = OffsetDateTime.of(2024, 10, 2, 12, 30, 30, 0, ZoneOffset.UTC);
+
+        // Test passing a date out from Python (wow, Python 2.7 timezone handling is primitive!)
+        Evaluable evaluable = new JythonEvaluable("from datetime import datetime,timedelta,tzinfo\n" +
+                "\n" +
+                "ZERO = timedelta(0)\n" +
+                "\n" +
+                "class UTC(tzinfo):\n" +
+                "    \"\"\"UTC\"\"\"\n" +
+                "\n" +
+                "    def utcoffset(self, dt):\n" +
+                "        return ZERO\n" +
+                "\n" +
+                "    def tzname(self, dt):\n" +
+                "        return \"UTC\"\n" +
+                "\n" +
+                "    def dst(self, dt):\n" +
+                "        return ZERO\n" +
+                "\n" +
+                "utc = UTC()" +
+                "\n" +
+                "return datetime(2024, 10, 2, 12, 30, 30, tzinfo=utc)");
+        Object result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, expectedDate);
+
+        // Test passing a date in
+        bindings.put("value", expectedDate);
+        evaluable = new JythonEvaluable("import datetime\n" +
+                "return isinstance(value,datetime.datetime)");
+        result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, 1L); // Python's version of true
+
+        evaluable = new JythonEvaluable("import datetime\n" +
+                "return str(value)");
+        result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, "2024-10-02 12:30:30");
+    }
+
+    @Test
+    public void testJythonDataTypes() {
+        Properties bindings = new Properties(); // tests are self-contained, so no bindings needed
+
+        // Test sequences
+        Evaluable evaluable = new JythonEvaluable("return (1,2)");
+        Object result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, new Long[] { 1L, 2L });
+
+        // Test floats
+        evaluable = new JythonEvaluable("return 1.2");
+        result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, 1.2);
+
+        // Test integers
+        evaluable = new JythonEvaluable("return 1");
+        result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, 1L);
+
+        // Test strings
+        evaluable = new JythonEvaluable("return 'foo'");
+        result = evaluable.evaluate(bindings);
+        Assert.assertEquals(result, "foo");
+
+    }
 }
diff --git a/main/tests/cypress/cypress/e2e/project/grid/column/facet/facets.numeric.cy.js b/main/tests/cypress/cypress/e2e/project/grid/column/facet/facets.numeric.cy.js
index 00d058978..839417216 100644
--- a/main/tests/cypress/cypress/e2e/project/grid/column/facet/facets.numeric.cy.js
+++ b/main/tests/cypress/cypress/e2e/project/grid/column/facet/facets.numeric.cy.js
@@ -38,23 +38,31 @@ describe(__filename, function () {
     cy.get('.slider-widget-bracket').eq(1)
     .trigger('mousedown',{ force: true })
     .trigger('mousemove',-130,0,{ force: true })
-    .trigger('mouseup',{ force: true })
+    .trigger('mouseup',{ force: true });
+    cy.get('.facet-range-status').contains('0 — 45');
     cy.get('#summary-bar').contains('72 matching rows');
 
-    //sliding the middle portion in numeric facet
+    // wait for the numeric facet to be refreshed before next mouse events
+    cy.get('.browsing-panel-indicator').should("not.be.visible");
+
+    // sliding entire selection by the body (middle)
     cy.get('.slider-widget-draggable').eq(0)
-      // FIXME: These are the last two uses of the hated waitForOrOperation
-    .trigger('mousedown',{ force: true }).waitForOrOperation()
+    .trigger('mousedown',{ force: true })
     .trigger('mousemove',130,0,{ force: true })
-    .trigger('mouseup',{ force: true }).waitForOrOperation();
-    cy.get('#summary-bar').contains('70 matching rows');
+    .trigger('mouseup',{ force: true });
+    cy.get('.facet-range-status').contains('23 — 68');
+    cy.get('#summary-bar').contains('74 matching rows');
+
+    // wait for the numeric facet to be refreshed before next mouse events
+    cy.get('.browsing-panel-indicator').should("not.be.visible");
 
     //sliding the left slider
     cy.get('.slider-widget-bracket').eq(0)
     .trigger('mousedown',{ force: true })
     .trigger('mousemove',130,0,{ force: true })
-    .trigger('mouseup',{ force: true })
-    cy.get('#summary-bar').contains('4 matching rows');
+    .trigger('mouseup',{ force: true });
+    cy.get('.facet-range-status').contains('66 — 68');
+    cy.get('#summary-bar').contains('3 matching rows');
   });
 
   it('Test for checkboxes and reset button', function () {
diff --git a/main/tests/cypress/cypress/support/commands.js b/main/tests/cypress/cypress/support/commands.js
index 306132747..72ccf187a 100644
--- a/main/tests/cypress/cypress/support/commands.js
+++ b/main/tests/cypress/cypress/support/commands.js
@@ -189,21 +189,6 @@ Cypress.Commands.add('navigateTo', (target) => {
   cy.get('#action-area-tabs li').contains(target).click();
 });
 
-/**
- * Wait for OpenRefine to finish an Ajax load
- *
- * @deprecated
- *
- * NOTE: This command is unreliable because if you call it after starting an operation e.g. with a click(), the loading
- * indicator may have come and gone already by the time waitForOrOperation() is called, causing the cypress test to
- * wait forever on ajax_in_progress=true until it fails due to timeout.
- *
- */
-Cypress.Commands.add('waitForOrOperation', () => {
-  cy.get('body[ajax_in_progress="true"]');
-  cy.get('body[ajax_in_progress="false"]');
-});
-
 /**
  * Utility method to fill something into the expression input
  */