Merge pull request #730 from finos/py-fix

Python improvements + refactor
finos · Sep 22, 2019 · b80912b · b80912b
2 parents a056023 + 81172af
commit b80912b
Show file tree

Hide file tree

Showing 34 changed files with 1,389 additions and 287 deletions.
diff --git a/.gitignore b/.gitignore
@@ -141,3 +141,5 @@ docs/static/img
 
 docs/static/arrow/
 .perspectiverc
+
+python/perspective/perspective/tests/table/psp_test
diff --git a/cpp/perspective/src/cpp/base.cpp b/cpp/perspective/src/cpp/base.cpp
@@ -328,12 +328,6 @@ filter_op_to_str(t_filter_op op) {
         case FILTER_OP_IS_NOT_NULL: {
             return "is not null";
         } break;
-        case FILTER_OP_IS_VALID: {
-            return "is not None";
-        } break;
-        case FILTER_OP_IS_NOT_VALID: {
-            return "is None";
-        } break;
     }
     PSP_COMPLAIN_AND_ABORT("Reached end of function");
     return "";
@@ -367,14 +361,10 @@ str_to_filter_op(const std::string& str) {
         return t_filter_op::FILTER_OP_AND;
     } else if (str == "|" || str == "or") {
         return t_filter_op::FILTER_OP_OR;
-    } else if (str == "is null") {
+    } else if (str == "is null" || str == "is None") {
         return t_filter_op::FILTER_OP_IS_NULL;
-    } else if (str == "is not null") {
+    } else if (str == "is not null" || str == "is not None") {
         return t_filter_op::FILTER_OP_IS_NOT_NULL;
-    } else if (str == "is not None") {
-        return t_filter_op::FILTER_OP_IS_VALID;
-    } else if (str == "is None") {
-        return t_filter_op::FILTER_OP_IS_NOT_VALID;
     } else {
         PSP_COMPLAIN_AND_ABORT("Encountered unknown filter operation.");
         // use and as default

diff --git a/cpp/perspective/src/cpp/pool.cpp b/cpp/perspective/src/cpp/pool.cpp
@@ -24,7 +24,8 @@ t_updctx::t_updctx(t_uindex gnode_id, const std::string& ctx)
     : m_gnode_id(gnode_id)
     , m_ctx(ctx) {}
 
-#ifdef PSP_ENABLE_WASM
+#if defined PSP_ENABLE_WASM
+
 t_val
 empty_callback() {
     t_val callback = t_val::global("Object").new_();
@@ -34,17 +35,29 @@ empty_callback() {
 
 t_pool::t_pool()
     : m_sleep(0)
-    , m_update_delegate(empty_callback())
-    , m_has_python_dep(false) {
-    m_run.clear();
+    , m_update_delegate(empty_callback()) {
+        m_run.clear();
+    }
+
+#elif defined PSP_ENABLE_PYTHON
+
+t_val
+empty_callback() {
+    return py::none();
 }
-#else
 
 t_pool::t_pool()
     : m_sleep(0)
-    , m_has_python_dep(false) {
-    m_run.clear();
-}
+    , m_update_delegate(empty_callback()) {
+        m_run.clear();
+    }
+
+#else
+
+t_pool::t_pool()
+    : m_sleep(0) {
+        m_run.clear();
+    }
 
 #endif
 
@@ -155,13 +168,6 @@ t_pool::set_sleep(t_uindex ms) {
     }
 }
 
-void
-t_pool::py_notify_userspace() {
-#ifdef PSP_ENABLE_WASM
-    m_update_delegate.call<void>("_update_callback");
-#endif
-}
-
 std::vector<t_stree*>
 t_pool::get_trees() {
     std::vector<t_stree*> rval;
@@ -180,13 +186,6 @@ t_pool::get_trees() {
     return rval;
 }
 
-#ifdef PSP_ENABLE_WASM
-void
-t_pool::set_update_delegate(t_val ud) {
-    m_update_delegate = ud;
-}
-#endif
-
 #ifdef PSP_ENABLE_WASM
 void
 t_pool::register_context(
@@ -195,31 +194,37 @@ t_pool::register_context(
     if (!validate_gnode_id(gnode_id))
         return;
     m_gnodes[gnode_id]->_register_context(name, type, ptr);
-
-    if (t_env::log_progress()) {
-        std::cout << repr() << " << t_pool.register_context: "
-                  << " gnode_id => " << gnode_id << " name => " << name << " type => " << type
-                  << " ptr => " << ptr << std::endl;
-    }
 }
 
 #else
-
 void
 t_pool::register_context(
     t_uindex gnode_id, const std::string& name, t_ctx_type type, std::int64_t ptr) {
     std::lock_guard<std::mutex> lg(m_mtx);
     if (!validate_gnode_id(gnode_id))
         return;
     m_gnodes[gnode_id]->_register_context(name, type, ptr);
+}
+#endif
 
-    if (t_env::log_progress()) {
-        std::cout << repr() << " << t_pool.register_context: "
-                  << " gnode_id => " << gnode_id << " name => " << name << " type => " << type
-                  << " ptr => " << ptr << std::endl;
-    }
+#if defined PSP_ENABLE_WASM || defined PSP_ENABLE_PYTHON
+void
+t_pool::set_update_delegate(t_val ud) {
+    m_update_delegate = ud;
 }
 #endif
+
+void
+t_pool::notify_userspace() {
+    #if defined PSP_ENABLE_WASM
+        m_update_delegate.call<void>("_update_callback");
+    #elif PSP_ENABLE_PYTHON
+        if (!m_update_delegate.is_none()) {
+            m_update_delegate.attr("_update_callback")();
+        }
+    #endif
+}
+
 void
 t_pool::unregister_context(t_uindex gnode_id, const std::string& name) {
     std::lock_guard<std::mutex> lg(m_mtx);

diff --git a/cpp/perspective/src/cpp/scalar.cpp b/cpp/perspective/src/cpp/scalar.cpp
@@ -1091,12 +1091,6 @@ t_tscalar::cmp(t_filter_op op, const t_tscalar& other) const {
         case FILTER_OP_IS_NOT_NULL: {
             return m_status == STATUS_VALID;
         } break;
-        case FILTER_OP_IS_VALID: {
-            return m_status == STATUS_VALID;
-        } break;
-        case FILTER_OP_IS_NOT_VALID: {
-            return m_status != STATUS_VALID;
-        } break;
         default: { PSP_COMPLAIN_AND_ABORT("Invalid filter op"); } break;
     }
 

diff --git a/cpp/perspective/src/cpp/update_task.cpp b/cpp/perspective/src/cpp/update_task.cpp
@@ -30,7 +30,7 @@ t_update_task::run() {
         }
         m_pool.m_data_remaining.store(false);
     }
-    m_pool.py_notify_userspace();
+    m_pool.notify_userspace();
     m_pool.inc_epoch();
 }
 
@@ -49,7 +49,7 @@ t_update_task::run(t_uindex gnode_id) {
         }
         m_pool.m_data_remaining.store(false);
     }
-    m_pool.py_notify_userspace();
+    m_pool.notify_userspace();
     m_pool.inc_epoch();
 }
 } // end namespace perspective
diff --git a/cpp/perspective/src/include/perspective/base.h b/cpp/perspective/src/include/perspective/base.h
@@ -184,9 +184,7 @@ enum t_filter_op {
     FILTER_OP_NOT_IN,
     FILTER_OP_AND,
     FILTER_OP_IS_NULL,
-    FILTER_OP_IS_NOT_NULL,
-    FILTER_OP_IS_VALID,
-    FILTER_OP_IS_NOT_VALID
+    FILTER_OP_IS_NOT_NULL
 };
 
 PERSPECTIVE_EXPORT std::string filter_op_to_str(t_filter_op op);

diff --git a/cpp/perspective/src/include/perspective/binding.h b/cpp/perspective/src/include/perspective/binding.h
@@ -310,10 +310,10 @@ namespace binding {
      *
      * Views are backed by an underlying `t_ctx_*` object, represented by the `CTX_T` template.
      *
-     * One-sided views have one or more `row-pivots` applied.
+     * One-sided views have one or more `row_pivots` applied.
      *
-     * Two sided views have one or more `row-pivots` and `column-pivots` applied, or they have
-     * one or more `column-pivots` applied without any row pivots, hence the term `column_only`.
+     * Two sided views have one or more `row_pivots` and `column_pivots` applied, or they have
+     * one or more `column_pivots` applied without any row pivots, hence the term `column_only`.
      *
      * @tparam T
      * @param table

diff --git a/cpp/perspective/src/include/perspective/pool.h b/cpp/perspective/src/include/perspective/pool.h
@@ -15,9 +15,12 @@
 #include <mutex>
 #include <atomic>
 
-#ifdef PSP_ENABLE_WASM
-#include <emscripten/val.h>
-typedef emscripten::val t_val;
+#if defined PSP_ENABLE_WASM
+    #include <emscripten/val.h>
+    typedef emscripten::val t_val;
+#elif defined PSP_ENABLE_PYTHON
+    #include <pybind11/pybind11.h>
+    typedef py::object t_val;
 #endif
 
 namespace perspective {
@@ -39,16 +42,20 @@ class PERSPECTIVE_EXPORT t_pool {
 public:
     t_pool();
     t_uindex register_gnode(t_gnode* node);
-#ifdef PSP_ENABLE_WASM
+
+#if defined PSP_ENABLE_WASM || defined PSP_ENABLE_PYTHON
     void set_update_delegate(t_val ud);
+#endif
+
+#ifdef PSP_ENABLE_WASM
     void register_context(
         t_uindex gnode_id, const std::string& name, t_ctx_type type, std::int32_t ptr);
-    void py_notify_userspace();
 #else
     void register_context(
         t_uindex gnode_id, const std::string& name, t_ctx_type type, std::int64_t ptr);
-    void py_notify_userspace();
 #endif
+
+    void notify_userspace();
     PSP_NON_COPYABLE(t_pool);
 
     ~t_pool();
@@ -94,14 +101,13 @@ class PERSPECTIVE_EXPORT t_pool {
     std::mutex m_mtx;
     std::vector<t_gnode*> m_gnodes;
 
-#ifdef PSP_ENABLE_WASM
+#if defined PSP_ENABLE_WASM || defined PSP_ENABLE_PYTHON
     t_val m_update_delegate;
 #endif
     std::atomic_flag m_run;
     std::atomic<bool> m_data_remaining;
     std::atomic<t_uindex> m_sleep;
     std::atomic<t_uindex> m_epoch;
-    bool m_has_python_dep;
 };
 
 } // end namespace perspective
diff --git a/examples/simple/superstore-hypergrid.html b/examples/simple/superstore-hypergrid.html
@@ -38,7 +38,7 @@
             xhr.responseType = "arraybuffer"
             xhr.onload = function() {
                 var el = document.getElementsByTagName('perspective-viewer')[0];
-                el.load(xhr.response);
+                el.load(xhr.response, {index: "Row ID"});
                 el.toggleConfig();
             }
             xhr.send(null);

diff --git a/examples/simple/test.html b/examples/simple/test.html
@@ -39,14 +39,11 @@
             // xhr.responseType = "arraybuffer"
             // xhr.onload = function() {
                 var el = document.getElementsByTagName('perspective-viewer')[0];
-                const table = perspective.shared_worker().table([{"a": "abc", "b": new Date(), "c": 3}], {index: "a"});
-                for (let i = 0; i < 100; i++) {
-
-                table.update([{"a": "abc", "b": ""}])
-                }
-                //table.update([{"a": 3, "b": 1}])
-                const v = table.view()
-                console.log(await v.to_json())
+                const d = {"x": [1, 2, 3], "y": ["a", "b", "c"]};
+                const table = perspective.shared_worker().table(d);
+                const v = table.view();
+                console.log(await v.to_columns({"index":true}))
+
             // }
             // xhr.send(null);
         });

diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js
@@ -1117,35 +1117,28 @@ export default function(Module) {
         return computed_schema;
     };
 
-    table.prototype._is_date_filter = function(schema) {
-        return key => schema[key] === "datetime" || schema[key] === "date";
-    };
-
-    table.prototype._is_valid_filter = function(filter) {
+    /**
+     * Validates a filter configuration, i.e. that the value to filter by is not null or undefined.
+     *
+     * @param {Array<string>} [filter] a filter configuration to test.
+     */
+    table.prototype.is_valid_filter = function(filter) {
         // isNull and isNotNull filter operators are always valid and apply to all schema types
         if (filter[1] === perspective.FILTER_OPERATORS.isNull || filter[1] === perspective.FILTER_OPERATORS.isNotNull) {
             return true;
         }
 
-        if (filter[2] === null) {
+        let value = filter[2];
+        if (value === null) {
             return false;
         }
 
         const schema = this.schema();
-        const isDateFilter = this._is_date_filter(schema);
-        const value = isDateFilter(filter[0]) ? new DateParser().parse(filter[2]) : filter[2];
-        return typeof value !== "undefined" && value !== null;
-    };
+        if (schema[filter[0]] === "date" || schema[filter[0]] === "datetime") {
+            value = new DateParser().parse(value);
+        }
 
-    /**
-     * Determines whether a given filter is valid.
-     *
-     * @param {Array<string>} [filter] A filter configuration array to test
-     *
-     * @returns {Promise<boolean>} Whether the filter is valid
-     */
-    table.prototype.is_valid_filter = function(filter) {
-        return this._is_valid_filter(filter);
+        return typeof value !== "undefined" && value !== null;
     };
 
     /**