From 8620755cc89483c29c035449b568d2406fbbb3d7 Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Wed, 3 Jun 2020 13:14:47 -0400
Subject: [PATCH 1/8] add internal class for manager wire API

---
 .../perspective/manager/manager.py            | 309 +----------------
 .../perspective/manager/manager_internal.py   | 321 ++++++++++++++++++
 2 files changed, 324 insertions(+), 306 deletions(-)
 create mode 100644 python/perspective/perspective/manager/manager_internal.py

diff --git a/python/perspective/perspective/manager/manager.py b/python/perspective/perspective/manager/manager.py
index d1ed7ba3d1..83adcd052e 100644
--- a/python/perspective/perspective/manager/manager.py
+++ b/python/perspective/perspective/manager/manager.py
@@ -6,36 +6,22 @@
 # the Apache License 2.0.  The full license can be found in the LICENSE file.
 #
 
-import logging
-import json
 import random
 import string
-import datetime
 from functools import partial
 from ..core.exception import PerspectiveError
 from ..table._callback_cache import _PerspectiveCallBackCache
-from ..table._date_validator import _PerspectiveDateValidator
-from ..table import Table, PerspectiveCppError
+from ..table import Table
 from ..table.view import View
 from .session import PerspectiveSession
-
-_date_validator = _PerspectiveDateValidator()
-
-
-class DateTimeEncoder(json.JSONEncoder):
-
-    def default(self, obj):
-        if isinstance(obj, datetime.datetime):
-            return _date_validator.to_timestamp(obj)
-        else:
-            return super(DateTimeEncoder, self).default(obj)
+from .manager_internal import _PerspectiveManagerInternal
 
 
 def gen_name(size=10, chars=string.ascii_uppercase + string.digits):
     return "".join(random.choice(chars) for x in range(size))
 
 
-class PerspectiveManager(object):
+class PerspectiveManager(_PerspectiveManagerInternal):
     '''PerspectiveManager is an orchestrator for running Perspective on the
     server side.
 
@@ -56,11 +42,6 @@ class PerspectiveManager(object):
         clean up associated resources.
     '''
 
-    # Commands that should be blocked from execution when the manager is in
-    # `locked` mode, i.e. its tables and views made immutable from remote
-    # modification.
-    LOCKED_COMMANDS = ["table", "update", "remove", "replace", "clear"]
-
     def __init__(self, lock=False):
         self._tables = {}
         self._views = {}
@@ -150,287 +131,3 @@ def _set_queue_process(self, func):
         for table in self._tables.values():
             table._state_manager.queue_process = partial(
                 self._queue_process_callback, state_manager=table._state_manager)
-
-    def _process(self, msg, post_callback, client_id=None):
-        '''Given a message from the client, process it through the Perspective
-        engine.
-
-        Args:
-            msg (:obj`dict`): a message from the client with instructions
-                that map to engine operations
-            post_callback (:obj`callable`): a function that returns data to the
-                client. `post_callback` should be a callable that takes two
-                parameters: `data` (str), and `binary` (bool), a kwarg that
-                specifies whether `data` is a binary string.
-        '''
-        if isinstance(msg, str):
-            if msg == "heartbeat":   # TODO fix this
-                return
-            msg = json.loads(msg)
-
-        if not isinstance(msg, dict):
-            raise PerspectiveError(
-                "Message passed into `_process` should either be a "
-                "JSON-serialized string or a dict.")
-
-        cmd = msg["cmd"]
-
-        if self._is_locked_command(msg) is True:
-            error_string = "`{0}` failed - access denied".format(
-                msg["cmd"] + (("." + msg["method"]) if msg.get("method", None) is not None else ""))
-            error_message = self._make_error_message(msg["id"], error_string)
-            post_callback(self._message_to_json(msg["id"], error_message))
-            return
-
-        try:
-            if cmd == "init":
-                # return empty response
-                message = self._make_message(msg["id"], None)
-                post_callback(self._message_to_json(msg["id"], message))
-            elif cmd == "table":
-                try:
-                    # create a new Table and track it
-                    data_or_schema = msg["args"][0]
-                    self._tables[msg["name"]] = Table(
-                        data_or_schema, **msg.get("options", {}))
-                except IndexError:
-                    self._tables[msg["name"]] = []
-            elif cmd == "view":
-                # create a new view and track it with the assigned client_id.
-                new_view = self._tables[msg["table_name"]].view(
-                    **msg.get("config", {}))
-                new_view._client_id = client_id
-                self._views[msg["view_name"]] = new_view
-            elif cmd == "table_method" or cmd == "view_method":
-                self._process_method_call(msg, post_callback, client_id)
-        except(PerspectiveError, PerspectiveCppError) as e:
-            # Catch errors and return them to client
-            error_message = self._make_error_message(msg["id"], str(e))
-            post_callback(self._message_to_json(msg["id"], error_message))
-
-    def _process_method_call(self, msg, post_callback, client_id):
-        '''When the client calls a method, validate the instance it calls on
-        and return the result.
-        '''
-        if msg["cmd"] == "table_method":
-            table_or_view = self._tables.get(msg["name"], None)
-        else:
-            table_or_view = self._views.get(msg["name"], None)
-            if table_or_view is None:
-                error_message = self._make_error_message(
-                    msg["id"], "View is not initialized")
-                post_callback(self._message_to_json(msg["id"], error_message))
-        try:
-            if msg.get("subscribe", False) is True:
-                self._process_subscribe(
-                    msg, table_or_view, post_callback, client_id)
-            else:
-                args = {}
-                if msg["method"] in ("schema", "computed_schema", "get_computation_input_types"):
-                    # make sure schema returns string types
-                    args["as_string"] = True
-                elif msg["method"].startswith("to_"):
-                    # parse options in `to_format` calls
-                    for d in msg.get("args", []):
-                        args.update(d)
-                else:
-                    args = msg.get("args", [])
-
-                if msg["method"] == "delete" and msg["cmd"] == "view_method":
-                    # views can be removed, but tables cannot
-                    self._views[msg["name"]].delete()
-                    self._views.pop(msg["name"], None)
-                    return
-
-                if msg["method"].startswith("to_"):
-                    # to_format takes dictionary of options
-                    result = getattr(table_or_view, msg["method"])(**args)
-                elif msg["method"] in ("update", "remove"):
-                    # Apply first arg as position, then options dict as kwargs
-                    data = args[0]
-                    options = {}
-                    if (len(args) > 1 and isinstance(args[1], dict)):
-                        options = args[1]
-                    result = getattr(table_or_view, msg["method"])(data, **options)
-                elif msg["method"] in ("computed_schema", "get_computation_input_types"):
-                    # these methods take args and kwargs
-                    result = getattr(table_or_view, msg["method"])(*msg.get("args", []), **args)
-                elif msg["method"] != "delete":
-                    # otherwise parse args as list
-                    result = getattr(table_or_view, msg["method"])(*args)
-                if isinstance(result, bytes) and msg["method"] != "to_csv":
-                    # return a binary to the client without JSON serialization,
-                    # i.e. when we return an Arrow. If a method is added that
-                    # returns a string, this condition needs to be updated as
-                    # an Arrow binary is both `str` and `bytes` in Python 2.
-                    self._process_bytes(result, msg, post_callback)
-                else:
-                    # return the result to the client
-                    message = self._make_message(msg["id"], result)
-                    post_callback(self._message_to_json(msg["id"], message))
-        except Exception as error:
-            message = self._make_error_message(msg["id"], str(error))
-            post_callback(self._message_to_json(msg["id"], message))
-
-    def _process_subscribe(self, msg, table_or_view, post_callback, client_id):
-        '''When the client attempts to add or remove a subscription callback,
-        validate and perform the requested operation.
-
-        Args:
-            msg (dict): the message from the client
-            table_or_view {Table|View} : the instance that the subscription
-                will be called on.
-            post_callback (callable): a method that notifies the client with
-                new data.
-            client_id (str) : a unique str id that identifies the
-                `PerspectiveSession` object that is passing the message.
-        '''
-        try:
-            callback = None
-            callback_id = msg.get("callback_id", None)
-            method = msg.get("method", None)
-            args = msg.get("args", [])
-            if method and method[:2] == "on":
-                # wrap the callback
-                callback = partial(
-                    self.callback, msg=msg, post_callback=post_callback)
-                if callback_id:
-                    self._callback_cache.add_callback({
-                        "client_id": client_id,
-                        "callback_id": callback_id,
-                        "callback": callback,
-                        "name": msg.get("name", None)
-                    })
-            elif callback_id is not None:
-                # remove the callback with `callback_id`
-                self._callback_cache.remove_callbacks(
-                    lambda cb: cb["callback_id"] != callback_id)
-            if callback is not None:
-                # call the underlying method on the Table or View, and apply
-                # the dictionary of kwargs that is passed through.
-                if (method == "on_update"):
-                    # If there are no arguments, make sure we call `on_update`
-                    # with mode set to "none".
-                    mode = {"mode": "none"}
-                    if len(args) > 0:
-                        mode = args[0]
-                    getattr(table_or_view, method)(callback, **mode)
-                else:
-                    getattr(table_or_view, method)(callback)
-            else:
-                logging.info("callback not found for remote call {}".format(msg))
-        except Exception as error:
-            message = self._make_error_message(msg["id"], str(error))
-            post_callback(self._message_to_json(msg["id"], message))
-
-    def _process_bytes(self, binary, msg, post_callback):
-        """Send a bytestring message to the client without attempting to
-        serialize as JSON.
-
-        Perspective's client expects two messages to be sent when a binary
-        payload is expected: the first message is a JSON-serialized string with
-        the message's `id` and `msg`, and the second message is a bytestring
-        without any additional metadata. Implementations of the `post_callback`
-        should have an optional kwarg named `binary`, which specifies whether
-        `data` is a bytestring or not.
-
-        Args:
-            binary (bytes, bytearray) : a byte message to be passed to the client.
-            msg (dict) : the original message that generated the binary
-                response from Perspective.
-            post_callback (callable) : a function that passes data to the
-                client, with a `binary` (bool) kwarg that allows it to pass
-                byte messages without serializing to JSON.
-        """
-        msg["is_transferable"] = True
-        post_callback(json.dumps(msg, cls=DateTimeEncoder))
-        post_callback(binary, binary=True)
-
-    def callback(self, *args, **kwargs):
-        '''Return a message to the client using the `post_callback` method.'''
-        id = kwargs.get("msg")["id"]
-        post_callback = kwargs.get("post_callback")
-        # Coerce the message to be an object so it can be handled in
-        # Javascript, where promises cannot be resolved with multiple args.
-        updated = {
-            "port_id": args[0],
-        }
-        msg = self._make_message(id, updated)
-        if len(args) > 1 and type(args[1]) == bytes:
-            self._process_bytes(args[1], msg, post_callback)
-        else:
-            post_callback(self._message_to_json(msg["id"], msg))
-
-    def clear_views(self, client_id):
-        '''Garbage collect views that belong to closed connections.'''
-        count = 0
-        names = []
-
-        if not client_id:
-            raise PerspectiveError("Cannot garbage collect views that are not linked to a specific client ID!")
-
-        for name, view in self._views.items():
-            if view._client_id == client_id:
-                view.delete()
-                names.append(name)
-                count += 1
-
-        for name in names:
-            self._views.pop(name)
-
-        logging.warning("GC {} views in memory".format(count))
-
-    def _make_message(self, id, result):
-        '''Return a serializable message for a successful result.'''
-        return {
-            "id": id,
-            "data": result
-        }
-
-    def _make_error_message(self, id, error):
-        '''Return a serializable message for an error result.'''
-        return {
-            "id": id,
-            "error": error
-        }
-
-    def _message_to_json(self, id, message):
-        '''Given a message object to be passed to Perspective, serialize it
-        into a string using `DateTimeEncoder` and `allow_nan=False`.
-
-        If an Exception occurs in serialization, catch the Exception and
-        return an error message using `self._make_error_message`.
-
-        Args:
-            message (:obj:`dict`) a serializable message to be passed to
-                Perspective.
-        '''
-        try:
-            return json.dumps(message, allow_nan=False, cls=DateTimeEncoder)
-        except ValueError as error:
-            error_string = str(error)
-
-            # Augment the default error message when invalid values are
-            # detected, i.e. `NaN`
-            if error_string == "Out of range float values are not JSON compliant":
-                error_string = "Cannot serialize `NaN`, `Infinity` or `-Infinity` to JSON."
-
-            error_message = self._make_error_message(id, "JSON serialization error: {}".format(error_string))
-
-            logging.warning(error_message["error"])
-            return json.dumps(error_message)
-
-    def _is_locked_command(self, msg):
-        '''Returns `True` if the manager instance is locked and the command
-        is in `PerspectiveManager.LOCKED_COMMANDS`, and `False` otherwise.'''
-        if not self._lock:
-            return False
-
-        cmd = msg["cmd"]
-        method = msg.get("method", None)
-
-        if cmd == "table_method" and method == "delete":
-            # table.delete is blocked, but view.delete is not
-            return True
-
-        return cmd == "table" or method in PerspectiveManager.LOCKED_COMMANDS
diff --git a/python/perspective/perspective/manager/manager_internal.py b/python/perspective/perspective/manager/manager_internal.py
new file mode 100644
index 0000000000..34133132c4
--- /dev/null
+++ b/python/perspective/perspective/manager/manager_internal.py
@@ -0,0 +1,321 @@
+################################################################################
+#
+# Copyright (c) 2019, the Perspective Authors.
+#
+# This file is part of the Perspective library, distributed under the terms of
+# the Apache License 2.0.  The full license can be found in the LICENSE file.
+#
+
+import datetime
+import logging
+import json
+from functools import partial
+from ..core.exception import PerspectiveError
+from ..table import Table, PerspectiveCppError
+from ..table._date_validator import _PerspectiveDateValidator
+
+_date_validator = _PerspectiveDateValidator()
+
+
+class DateTimeEncoder(json.JSONEncoder):
+    """Before sending datetimes over the wire, convert them to Unix timestamps
+    in milliseconds since epoch, using Perspective's date validator to
+    ensure consistency."""
+
+    def default(self, obj):
+        if isinstance(obj, datetime.datetime):
+            return _date_validator.to_timestamp(obj)
+        else:
+            return super(DateTimeEncoder, self).default(obj)
+
+
+class _PerspectiveManagerInternal(object):
+
+    # Commands that should be blocked from execution when the manager is in
+    # `locked` mode, i.e. its tables and views made immutable from remote
+    # modification.
+    LOCKED_COMMANDS = ["table", "update", "remove", "replace", "clear"]
+
+    def _process(self, msg, post_callback, client_id=None):
+        '''Given a message from the client, process it through the Perspective
+        engine.
+
+        Args:
+            msg (:obj`dict`): a message from the client with instructions
+                that map to engine operations
+            post_callback (:obj`callable`): a function that returns data to the
+                client. `post_callback` should be a callable that takes two
+                parameters: `data` (str), and `binary` (bool), a kwarg that
+                specifies whether `data` is a binary string.
+        '''
+        if isinstance(msg, str):
+            if msg == "heartbeat":   # TODO fix this
+                return
+            msg = json.loads(msg)
+
+        if not isinstance(msg, dict):
+            raise PerspectiveError(
+                "Message passed into `_process` should either be a "
+                "JSON-serialized string or a dict.")
+
+        cmd = msg["cmd"]
+
+        if self._is_locked_command(msg) is True:
+            error_string = "`{0}` failed - access denied".format(
+                msg["cmd"] + (("." + msg["method"]) if msg.get("method", None) is not None else ""))
+            error_message = self._make_error_message(msg["id"], error_string)
+            post_callback(self._message_to_json(msg["id"], error_message))
+            return
+
+        try:
+            if cmd == "init":
+                # return empty response
+                message = self._make_message(msg["id"], None)
+                post_callback(self._message_to_json(msg["id"], message))
+            elif cmd == "table":
+                try:
+                    # create a new Table and track it
+                    data_or_schema = msg["args"][0]
+                    self._tables[msg["name"]] = Table(
+                        data_or_schema, **msg.get("options", {}))
+                except IndexError:
+                    self._tables[msg["name"]] = []
+            elif cmd == "view":
+                # create a new view and track it with the assigned client_id.
+                new_view = self._tables[msg["table_name"]].view(
+                    **msg.get("config", {}))
+                new_view._client_id = client_id
+                self._views[msg["view_name"]] = new_view
+            elif cmd == "table_method" or cmd == "view_method":
+                self._process_method_call(msg, post_callback, client_id)
+        except(PerspectiveError, PerspectiveCppError) as e:
+            # Catch errors and return them to client
+            error_message = self._make_error_message(msg["id"], str(e))
+            post_callback(self._message_to_json(msg["id"], error_message))
+
+    def _process_method_call(self, msg, post_callback, client_id):
+        '''When the client calls a method, validate the instance it calls on
+        and return the result.
+        '''
+        if msg["cmd"] == "table_method":
+            table_or_view = self._tables.get(msg["name"], None)
+        else:
+            table_or_view = self._views.get(msg["name"], None)
+            if table_or_view is None:
+                error_message = self._make_error_message(
+                    msg["id"], "View is not initialized")
+                post_callback(self._message_to_json(msg["id"], error_message))
+        try:
+            if msg.get("subscribe", False) is True:
+                self._process_subscribe(
+                    msg, table_or_view, post_callback, client_id)
+            else:
+                args = {}
+                if msg["method"] in ("schema", "computed_schema", "get_computation_input_types"):
+                    # make sure schema returns string types
+                    args["as_string"] = True
+                elif msg["method"].startswith("to_"):
+                    # parse options in `to_format` calls
+                    for d in msg.get("args", []):
+                        args.update(d)
+                else:
+                    args = msg.get("args", [])
+
+                if msg["method"] == "delete" and msg["cmd"] == "view_method":
+                    # views can be removed, but tables cannot
+                    self._views[msg["name"]].delete()
+                    self._views.pop(msg["name"], None)
+                    return
+
+                if msg["method"].startswith("to_"):
+                    # to_format takes dictionary of options
+                    result = getattr(table_or_view, msg["method"])(**args)
+                elif msg["method"] in ("update", "remove"):
+                    # Apply first arg as position, then options dict as kwargs
+                    data = args[0]
+                    options = {}
+                    if (len(args) > 1 and isinstance(args[1], dict)):
+                        options = args[1]
+                    result = getattr(table_or_view, msg["method"])(data, **options)
+                elif msg["method"] in ("computed_schema", "get_computation_input_types"):
+                    # these methods take args and kwargs
+                    result = getattr(table_or_view, msg["method"])(*msg.get("args", []), **args)
+                elif msg["method"] != "delete":
+                    # otherwise parse args as list
+                    result = getattr(table_or_view, msg["method"])(*args)
+                if isinstance(result, bytes) and msg["method"] != "to_csv":
+                    # return a binary to the client without JSON serialization,
+                    # i.e. when we return an Arrow. If a method is added that
+                    # returns a string, this condition needs to be updated as
+                    # an Arrow binary is both `str` and `bytes` in Python 2.
+                    self._process_bytes(result, msg, post_callback)
+                else:
+                    # return the result to the client
+                    message = self._make_message(msg["id"], result)
+                    post_callback(self._message_to_json(msg["id"], message))
+        except Exception as error:
+            message = self._make_error_message(msg["id"], str(error))
+            post_callback(self._message_to_json(msg["id"], message))
+
+    def _process_subscribe(self, msg, table_or_view, post_callback, client_id):
+        '''When the client attempts to add or remove a subscription callback,
+        validate and perform the requested operation.
+
+        Args:
+            msg (dict): the message from the client
+            table_or_view {Table|View} : the instance that the subscription
+                will be called on.
+            post_callback (callable): a method that notifies the client with
+                new data.
+            client_id (str) : a unique str id that identifies the
+                `PerspectiveSession` object that is passing the message.
+        '''
+        try:
+            callback = None
+            callback_id = msg.get("callback_id", None)
+            method = msg.get("method", None)
+            args = msg.get("args", [])
+            if method and method[:2] == "on":
+                # wrap the callback
+                callback = partial(
+                    self.callback, msg=msg, post_callback=post_callback)
+                if callback_id:
+                    self._callback_cache.add_callback({
+                        "client_id": client_id,
+                        "callback_id": callback_id,
+                        "callback": callback,
+                        "name": msg.get("name", None)
+                    })
+            elif callback_id is not None:
+                # remove the callback with `callback_id`
+                self._callback_cache.remove_callbacks(
+                    lambda cb: cb["callback_id"] != callback_id)
+            if callback is not None:
+                # call the underlying method on the Table or View, and apply
+                # the dictionary of kwargs that is passed through.
+                if (method == "on_update"):
+                    # If there are no arguments, make sure we call `on_update`
+                    # with mode set to "none".
+                    mode = {"mode": "none"}
+                    if len(args) > 0:
+                        mode = args[0]
+                    getattr(table_or_view, method)(callback, **mode)
+                else:
+                    getattr(table_or_view, method)(callback)
+            else:
+                logging.info("callback not found for remote call {}".format(msg))
+        except Exception as error:
+            message = self._make_error_message(msg["id"], str(error))
+            post_callback(self._message_to_json(msg["id"], message))
+
+    def _process_bytes(self, binary, msg, post_callback):
+        """Send a bytestring message to the client without attempting to
+        serialize as JSON.
+
+        Perspective's client expects two messages to be sent when a binary
+        payload is expected: the first message is a JSON-serialized string with
+        the message's `id` and `msg`, and the second message is a bytestring
+        without any additional metadata. Implementations of the `post_callback`
+        should have an optional kwarg named `binary`, which specifies whether
+        `data` is a bytestring or not.
+
+        Args:
+            binary (bytes, bytearray) : a byte message to be passed to the client.
+            msg (dict) : the original message that generated the binary
+                response from Perspective.
+            post_callback (callable) : a function that passes data to the
+                client, with a `binary` (bool) kwarg that allows it to pass
+                byte messages without serializing to JSON.
+        """
+        msg["is_transferable"] = True
+        post_callback(json.dumps(msg, cls=DateTimeEncoder))
+        post_callback(binary, binary=True)
+
+    def callback(self, *args, **kwargs):
+        '''Return a message to the client using the `post_callback` method.'''
+        id = kwargs.get("msg")["id"]
+        post_callback = kwargs.get("post_callback")
+        # Coerce the message to be an object so it can be handled in
+        # Javascript, where promises cannot be resolved with multiple args.
+        updated = {
+            "port_id": args[0],
+        }
+        msg = self._make_message(id, updated)
+        if len(args) > 1 and type(args[1]) == bytes:
+            self._process_bytes(args[1], msg, post_callback)
+        else:
+            post_callback(self._message_to_json(msg["id"], msg))
+
+    def clear_views(self, client_id):
+        '''Garbage collect views that belong to closed connections.'''
+        count = 0
+        names = []
+
+        if not client_id:
+            raise PerspectiveError("Cannot garbage collect views that are not linked to a specific client ID!")
+
+        for name, view in self._views.items():
+            if view._client_id == client_id:
+                view.delete()
+                names.append(name)
+                count += 1
+
+        for name in names:
+            self._views.pop(name)
+
+        logging.warning("GC {} views in memory".format(count))
+
+    def _make_message(self, id, result):
+        '''Return a serializable message for a successful result.'''
+        return {
+            "id": id,
+            "data": result
+        }
+
+    def _make_error_message(self, id, error):
+        '''Return a serializable message for an error result.'''
+        return {
+            "id": id,
+            "error": error
+        }
+
+    def _message_to_json(self, id, message):
+        '''Given a message object to be passed to Perspective, serialize it
+        into a string using `DateTimeEncoder` and `allow_nan=False`.
+
+        If an Exception occurs in serialization, catch the Exception and
+        return an error message using `self._make_error_message`.
+
+        Args:
+            message (:obj:`dict`) a serializable message to be passed to
+                Perspective.
+        '''
+        try:
+            return json.dumps(message, allow_nan=False, cls=DateTimeEncoder)
+        except ValueError as error:
+            error_string = str(error)
+
+            # Augment the default error message when invalid values are
+            # detected, i.e. `NaN`
+            if error_string == "Out of range float values are not JSON compliant":
+                error_string = "Cannot serialize `NaN`, `Infinity` or `-Infinity` to JSON."
+
+            error_message = self._make_error_message(id, "JSON serialization error: {}".format(error_string))
+
+            logging.warning(error_message["error"])
+            return json.dumps(error_message)
+
+    def _is_locked_command(self, msg):
+        '''Returns `True` if the manager instance is locked and the command
+        is in `_PerspectiveManagerInternal.LOCKED_COMMANDS`, and `False` otherwise.'''
+        if not self._lock:
+            return False
+
+        cmd = msg["cmd"]
+        method = msg.get("method", None)
+
+        if cmd == "table_method" and method == "delete":
+            # table.delete is blocked, but view.delete is not
+            return True
+
+        return cmd == "table" or method in _PerspectiveManagerInternal.LOCKED_COMMANDS

From 8f1a5d1bbefa02713702f3e8431404b579e56c1a Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Thu, 4 Jun 2020 16:10:41 -0400
Subject: [PATCH 2/8] add wire API tests for session, on_update, remove_update,
 make sure that manager remove callbacks removes callbacks from underlying
 view/table

---
 .../perspective/manager/manager.py            |  20 +-
 .../perspective/manager/manager_internal.py   |  17 +-
 .../perspective/manager/session.py            |   4 +-
 .../perspective/table/_callback_cache.py      |  30 +-
 python/perspective/perspective/table/table.py |   6 +-
 python/perspective/perspective/table/view.py  |   8 +-
 .../perspective/tests/manager/test_manager.py | 279 +++++++++++++++++-
 .../perspective/tests/manager/test_session.py | 246 +++++++++++++++
 .../perspective/tests/table/test_view.py      |   2 +-
 9 files changed, 585 insertions(+), 27 deletions(-)
 create mode 100644 python/perspective/perspective/tests/manager/test_session.py

diff --git a/python/perspective/perspective/manager/manager.py b/python/perspective/perspective/manager/manager.py
index 83adcd052e..5545250cd8 100644
--- a/python/perspective/perspective/manager/manager.py
+++ b/python/perspective/perspective/manager/manager.py
@@ -10,7 +10,6 @@
 import string
 from functools import partial
 from ..core.exception import PerspectiveError
-from ..table._callback_cache import _PerspectiveCallBackCache
 from ..table import Table
 from ..table.view import View
 from .session import PerspectiveSession
@@ -26,15 +25,16 @@ class PerspectiveManager(_PerspectiveManagerInternal):
     server side.
 
     The core functionality resides in `process()`, which receives
-    JSON-serialized messages from a client (usually `perspective-viewer` in the
-    browser), executes the commands in the message, and returns the results of
-    those commands back to the `post_callback`.  The manager cannot create
-    tables or views - use `host_table` or `host_view` to pass Table/View
-    instances to the manager.  Because Perspective is designed to be used in a
+    JSON-serialized messages from a client (implemented by `perspective-viewer`
+    in the browser), executes the commands in the message, and returns the
+    results of those commands back to the `post_callback`. Table/View instances
+    should be passed to the manager using `host_table` or `host_view` - this
+    allows server code to call Table/View APIs natively instead of proxying
+    them through the Manager. Because Perspective is designed to be used in a
     shared context, i.e. multiple clients all accessing the same `Table`,
     PerspectiveManager comes with the context of `sessions` - an
     encapsulation of the actions and resources used by a single connection
-    to Perspective.
+    to Perspective, which can be cleaned up after the connection is closed.
 
     - When a client connects, for example through a websocket, a new session
         should be spawned using `new_session()`.
@@ -43,11 +43,7 @@ class PerspectiveManager(_PerspectiveManagerInternal):
     '''
 
     def __init__(self, lock=False):
-        self._tables = {}
-        self._views = {}
-        self._callback_cache = _PerspectiveCallBackCache()
-        self._queue_process_callback = None
-        self._lock = lock
+        super(PerspectiveManager, self).__init__(lock=lock)
 
     def lock(self):
         """Block messages that can mutate the state of :obj:`~perspective.Table`
diff --git a/python/perspective/perspective/manager/manager_internal.py b/python/perspective/perspective/manager/manager_internal.py
index 34133132c4..cef520933f 100644
--- a/python/perspective/perspective/manager/manager_internal.py
+++ b/python/perspective/perspective/manager/manager_internal.py
@@ -12,6 +12,7 @@
 from functools import partial
 from ..core.exception import PerspectiveError
 from ..table import Table, PerspectiveCppError
+from ..table._callback_cache import _PerspectiveCallBackCache
 from ..table._date_validator import _PerspectiveDateValidator
 
 _date_validator = _PerspectiveDateValidator()
@@ -36,6 +37,13 @@ class _PerspectiveManagerInternal(object):
     # modification.
     LOCKED_COMMANDS = ["table", "update", "remove", "replace", "clear"]
 
+    def __init__(self, lock=False):
+        self._tables = {}
+        self._views = {}
+        self._callback_cache = _PerspectiveCallBackCache()
+        self._queue_process_callback = None
+        self._lock = lock
+
     def _process(self, msg, post_callback, client_id=None):
         '''Given a message from the client, process it through the Perspective
         engine.
@@ -187,9 +195,12 @@ def _process_subscribe(self, msg, table_or_view, post_callback, client_id):
                         "name": msg.get("name", None)
                     })
             elif callback_id is not None:
-                # remove the callback with `callback_id`
-                self._callback_cache.remove_callbacks(
-                    lambda cb: cb["callback_id"] != callback_id)
+                # pop the callback from the cache of the manager, and
+                # remove each of them from the underlying table or view
+                popped_callbacks = self._callback_cache.pop_callbacks(callback_id)
+
+                for callback in popped_callbacks:
+                    getattr(table_or_view, method)(callback["callback"])
             if callback is not None:
                 # call the underlying method on the Table or View, and apply
                 # the dictionary of kwargs that is passed through.
diff --git a/python/perspective/perspective/manager/session.py b/python/perspective/perspective/manager/session.py
index 88a8d9573e..ca4d4c9502 100644
--- a/python/perspective/perspective/manager/session.py
+++ b/python/perspective/perspective/manager/session.py
@@ -44,11 +44,11 @@ def close(self):
 
     def _clear_callbacks(self):
         # remove all callbacks from the view's cache
-        for cb in self.manager._callback_cache.get_callbacks():
+        for cb in self.manager._callback_cache:
             if cb["client_id"] == self.client_id:
                 view = self.manager.get_view(cb["name"])
                 if view:
                     view.remove_update(cb["callback"])
         # remove all callbacks from the manager's cache
         self.manager._callback_cache.remove_callbacks(
-            lambda cb: cb["client_id"] != self.client_id)
+            lambda cb: cb["client_id"] == self.client_id)
diff --git a/python/perspective/perspective/table/_callback_cache.py b/python/perspective/perspective/table/_callback_cache.py
index ce09654eaa..c165ca59a1 100644
--- a/python/perspective/perspective/table/_callback_cache.py
+++ b/python/perspective/perspective/table/_callback_cache.py
@@ -23,10 +23,38 @@ def remove_callbacks(self, condition):
         '''
         if not callable(condition):
             raise ValueError("callback filter condition must be a callable function!")
-        self._callbacks = [callback for callback in self._callbacks if condition(callback) is True]
+        self._callbacks = [callback for callback in self._callbacks if condition(callback) is False]
+
+    def pop_callbacks(self, callback_id):
+        """Removes and returns a list of callbacks with the given
+        `callback_id`.
+
+        Args:
+            callback_id (:obj:`str`) an id that identifies the callback.
+
+        Returns:
+            :obj:`list` a list of dicts containing the callbacks that were
+                removed.
+        """
+        popped = []
+        new_callbacks = []
+
+        for callback in self._callbacks:
+            if callback["callback_id"] == callback_id:
+                popped.append(callback)
+            else:
+                new_callbacks.append(callback)
+
+        return popped
 
     def get_callbacks(self):
         return self._callbacks
 
+    def __iter__(self):
+        return iter(self._callbacks)
+
+    def __len__(self):
+        return len(self._callbacks)
+
     def __repr__(self):
         return str(self._callbacks)
diff --git a/python/perspective/perspective/table/table.py b/python/perspective/perspective/table/table.py
index 43e2620d3d..161f2c52b5 100644
--- a/python/perspective/perspective/table/table.py
+++ b/python/perspective/perspective/table/table.py
@@ -441,7 +441,7 @@ def remove_delete(self, callback):
         if not callable(callback):
             return ValueError(
                 "remove_delete callback should be a callable function!")
-        self._delete_callbacks.remove_callbacks(lambda cb: cb != callback)
+        self._delete_callbacks.remove_callbacks(lambda cb: cb == callback)
 
     def delete(self):
         '''Delete this :class:`~perspective.Table` and clean up associated
@@ -454,7 +454,7 @@ def delete(self):
                 "- call delete() on each view, and try again.")
         self._state_manager.remove_process(self._table.get_id())
         self._table.unregister_gnode(self._gnode_id)
-        [cb() for cb in self._delete_callbacks.get_callbacks()]
+        [cb() for cb in self._delete_callbacks]
 
     def _update_callback(self, port_id):
         """After `process` completes internally, this method is called by the
@@ -466,5 +466,5 @@ def _update_callback(self, port_id):
             came from.
         """
         cache = {}
-        for callback in self._callbacks.get_callbacks():
+        for callback in self._callbacks:
             callback["callback"](port_id=port_id, cache=cache)
diff --git a/python/perspective/perspective/table/view.py b/python/perspective/perspective/table/view.py
index 3d06a1cb2a..1614cdeb11 100644
--- a/python/perspective/perspective/table/view.py
+++ b/python/perspective/perspective/table/view.py
@@ -255,7 +255,7 @@ def remove_update(self, callback):
         self._table._state_manager.call_process(self._table._table.get_id())
         if not callable(callback):
             return ValueError("remove_update callback should be a callable function!")
-        self._callbacks.remove_callbacks(lambda cb: cb["orig_callback"] != callback)
+        self._callbacks.remove_callbacks(lambda cb: cb["orig_callback"] == callback)
 
     def on_delete(self, callback):
         '''Set a callback to be run when the :func:`perspective.View.delete()`
@@ -292,8 +292,8 @@ def delete(self):
         self._table._state_manager.remove_process(self._table._table.get_id())
         self._table._views.pop(self._table._views.index(self._name))
         # remove the callbacks associated with this view
-        self._callbacks.remove_callbacks(lambda cb: cb["name"] != self._name)
-        [cb() for cb in self._delete_callbacks.get_callbacks()]
+        self._callbacks.remove_callbacks(lambda cb: cb["name"] == self._name)
+        [cb() for cb in self._delete_callbacks]
 
     def remove_delete(self, callback):
         '''Remove the delete callback associated with this
@@ -318,7 +318,7 @@ def remove_delete(self, callback):
         '''
         if not callable(callback):
             return ValueError("remove_delete callback should be a callable function!")
-        self._delete_callbacks.remove_callbacks(lambda cb: cb != callback)
+        self._delete_callbacks.remove_callbacks(lambda cb: cb == callback)
 
     def to_arrow(self, **kwargs):
         options = _parse_format_options(self, kwargs)
diff --git a/python/perspective/perspective/tests/manager/test_manager.py b/python/perspective/perspective/tests/manager/test_manager.py
index 40a64422dd..2d237bc97e 100644
--- a/python/perspective/perspective/tests/manager/test_manager.py
+++ b/python/perspective/perspective/tests/manager/test_manager.py
@@ -23,8 +23,8 @@ class TestPerspectiveManager(object):
     def post(self, msg):
         '''boilerplate callback to simulate a client's `post()` method.'''
         msg = json.loads(msg)
+        print("self.post:", msg)
         assert msg["id"] is not None
-        print(msg)
 
     def validate_post(self, msg, expected=None):
         msg = json.loads(msg)
@@ -581,6 +581,238 @@ def update_callback(port_id, delta):
         manager._process(update2, self.post)
         assert s.get() == 0
 
+    def test_manager_on_update_through_wire_API(self, sentinel):
+        s = sentinel(0)
+
+        # create a table and view using manager
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager = PerspectiveManager()
+        manager._process(make_table, self.post)
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        manager._process(make_view, self.post)
+
+        def callback(updated):
+            assert updated["port_id"] == 0
+            s.set(s.get() + 100)
+
+        # simulate a client that holds callbacks by id
+        callbacks = {
+            3: callback
+        }
+
+        def post_update(msg):
+            # when `on_update` is triggered, this callback gets the message
+            # and has to decide which callback to trigger.
+            message = json.loads(msg)
+            assert message["id"] is not None
+            if message["id"] == 3:
+                # trigger callback
+                assert message["data"] == {
+                    "port_id": 0
+                }
+                callbacks[message["id"]](message["data"])
+
+        # hook into the created view and pass it the callback
+        make_on_update = {"id": 3, "name": "view1", "cmd": "view_method", "subscribe": True, "method": "on_update", "callback_id": "callback_1"}
+        manager._process(make_on_update, post_update)
+
+        # call updates
+        update1 = {"id": 4, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [4], "b": ["d"]}]}
+        update2 = {"id": 5, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [5], "b": ["e"]}]}
+        manager._process(update1, self.post)
+        manager._process(update2, self.post)
+        assert s.get() == 200
+
+    def test_manager_on_update_rows_through_wire_API(self, sentinel):
+        s = sentinel(0)
+
+        # create a table and view using manager
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager = PerspectiveManager()
+        manager._process(make_table, self.post)
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        manager._process(make_view, self.post)
+
+        def callback(delta):
+            table = Table(delta)
+            assert table.size() == 1
+            assert table.schema() == {
+                "a": int,
+                "b": str
+            }
+            table.delete()
+            s.set(s.get() + 100)
+
+        # simulate a client that holds callbacks by id
+        callbacks = {
+            3: callback
+        }
+
+        def post_update(msg, binary=False):
+            # when `on_update` is triggered, this callback gets the message
+            # and has to decide which callback to trigger.
+            if binary:
+                # trigger callback - "msg" here is binary
+                callbacks[3](msg)
+                return
+
+            message = json.loads(msg)
+            assert message["id"] is not None
+            if message["id"] == 3:
+                # wait for transferable
+                assert message["data"]["port_id"] == 0
+                return
+
+        # hook into the created view and pass it the callback
+        make_on_update = {
+            "id": 3,
+            "name": "view1",
+            "cmd": "view_method",
+            "subscribe": True,
+            "method": "on_update",
+            "callback_id": "callback_1",
+            "args": [{"mode": "row"}]
+        }
+        manager._process(make_on_update, post_update)
+
+        # call updates
+        update1 = {"id": 4, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [4], "b": ["d"]}]}
+        update2 = {"id": 5, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [5], "b": ["e"]}]}
+        manager._process(update1, self.post)
+        manager._process(update2, self.post)
+        assert s.get() == 200
+
+    def test_manager_on_update_rows_with_port_id_through_wire_API(self, sentinel):
+        s = sentinel(0)
+
+        def update_callback(port_id, delta):
+            table = Table(delta)
+            assert table.size() == 1
+            assert table.schema() == {
+                "a": int,
+                "b": str
+            }
+            table.delete()
+            s.set(s.get() + 1)
+
+        # create a table and view using manager
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager = PerspectiveManager()
+        manager._process(make_table, self.post)
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        manager._process(make_view, self.post)
+
+        # Get two ports on the table
+        make_port = {"id": 3, "name": "table1", "cmd": "table_method", "method": "make_port"}
+        make_port2 = {"id": 4, "name": "table1", "cmd": "table_method", "method": "make_port"}
+
+        manager._process(make_port, self.post)
+        manager._process(make_port2, self.post)
+
+        # define an update callback
+        def callback(delta):
+            table = Table(delta)
+            assert table.size() == 1
+            assert table.schema() == {
+                "a": int,
+                "b": str
+            }
+            table.delete()
+            s.set(s.get() + 100)
+
+        # simulate a client that holds callbacks by id
+        callbacks = {
+            3: callback
+        }
+
+        def post_update(msg, binary=False):
+            # when `on_update` is triggered, this callback gets the message
+            # and has to decide which callback to trigger.
+            if binary:
+                # trigger callback - "msg" here is binary
+                callbacks[3](msg)
+                return
+
+            message = json.loads(msg)
+
+            assert message["id"] is not None
+
+            if message["id"] == 3:
+                # wait for transferable
+                assert message["data"]["port_id"] in (1, 2)
+                return
+
+        # hook into the created view and pass it the callback
+        make_on_update = {
+            "id": 5,
+            "name": "view1",
+            "cmd": "view_method",
+            "subscribe": True,
+            "method": "on_update",
+            "callback_id": "callback_1",
+            "args": [{"mode": "row"}]
+        }
+        manager._process(make_on_update, post_update)
+
+        # call updates
+        update1 = {"id": 6, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [4], "b": ["d"]}, {"port_id": 1}]}
+        update2 = {"id": 7, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [5], "b": ["e"]}, {"port_id": 2}]}
+
+        manager._process(update1, self.post)
+        manager._process(update2, self.post)
+
+        assert s.get() == 200
+
+    def test_manager_remove_update_through_wire_API(self, sentinel):
+        s = sentinel(0)
+
+        def update_callback(port_id, delta):
+            s.set(s.get() + 1)
+
+        # create a table and view using manager
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager = PerspectiveManager()
+        manager._process(make_table, self.post)
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        manager._process(make_view, self.post)
+
+        def callback(updated):
+            assert updated["port_id"] == 0
+            s.set(s.get() + 100)
+
+        # simulate a client that holds callbacks by id
+        callbacks = {
+            3: callback
+        }
+
+        def post_update(msg):
+            # when `on_update` is triggered, this callback gets the message
+            # and has to decide which callback to trigger.
+            message = json.loads(msg)
+            assert message["id"] is not None
+            if message["id"] == 3:
+                # trigger callback
+                assert message["data"] == {
+                    "port_id": 0
+                }
+                callbacks[message["id"]](message["data"])
+
+        # create an on_update callback
+        make_on_update = {"id": 3, "name": "view1", "cmd": "view_method", "subscribe": True, "method": "on_update", "callback_id": "callback_1"}
+        manager._process(make_on_update, post_update)
+
+        # call updates
+        update1 = {"id": 4, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [4], "b": ["d"]}]}
+        manager._process(update1, self.post)
+
+        # remove update callback
+        remove_on_update = {"id": 5, "name": "view1", "cmd": "view_method", "subscribe": True, "method": "remove_update", "callback_id": "callback_1"}
+        manager._process(remove_on_update, self.post)
+
+        update2 = {"id": 6, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [5], "b": ["e"]}]}
+        manager._process(update2, self.post)
+        assert s.get() == 100
+
     def test_manager_delete_view(self):
         make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
         manager = PerspectiveManager()
@@ -591,6 +823,51 @@ def test_manager_delete_view(self):
         manager._process(delete_view, self.post)
         assert len(manager._views) == 0
 
+    def test_manager_on_delete_view(self, sentinel):
+        s = sentinel(False)
+
+        def delete_callback():
+            s.set(True)
+
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager = PerspectiveManager()
+        manager._process(make_table, self.post)
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        manager._process(make_view, self.post)
+
+        view = manager.get_view("view1")
+        view.on_delete(delete_callback)
+
+        delete_view = {"id": 3, "name": "view1", "cmd": "view_method", "method": "delete"}
+
+        manager._process(delete_view, self.post)
+
+        assert len(manager._views) == 0
+        assert s.get() is True
+
+    def test_manager_remove_delete_view(self, sentinel):
+        s = sentinel(False)
+
+        def delete_callback():
+            s.set(True)
+
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager = PerspectiveManager()
+        manager._process(make_table, self.post)
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        manager._process(make_view, self.post)
+
+        view = manager.get_view("view1")
+        view.on_delete(delete_callback)
+        view.remove_delete(delete_callback)
+
+        delete_view = {"id": 3, "name": "view1", "cmd": "view_method", "method": "delete"}
+
+        manager._process(delete_view, self.post)
+
+        assert len(manager._views) == 0
+        assert s.get() is False
+
     def test_manager_set_queue_process(self, sentinel):
         s = sentinel(0)
         manager = PerspectiveManager()
diff --git a/python/perspective/perspective/tests/manager/test_session.py b/python/perspective/perspective/tests/manager/test_session.py
new file mode 100644
index 0000000000..623cdb95a0
--- /dev/null
+++ b/python/perspective/perspective/tests/manager/test_session.py
@@ -0,0 +1,246 @@
+################################################################################
+#
+# Copyright (c) 2019, the Perspective Authors.
+#
+# This file is part of the Perspective library, distributed under the terms of
+# the Apache License 2.0.  The full license can be found in the LICENSE file.
+#
+
+import json
+import random
+from perspective import Table, PerspectiveManager
+
+data = {"a": [1, 2, 3], "b": ["a", "b", "c"]}
+
+
+class TestPerspectiveSession(object):
+
+    def post(self, msg):
+        '''boilerplate callback to simulate a client's `post()` method.'''
+        msg = json.loads(msg)
+        print("self.post: ", msg)
+        assert msg["id"] is not None
+
+    def validate_post(self, msg, expected=None):
+        msg = json.loads(msg)
+        if expected:
+            assert msg == expected
+
+    # test session
+    def test_session_new_session(self, sentinel):
+        s = sentinel(False)
+
+        def handle_to_dict(msg):
+            s.set(True)
+            message = json.loads(msg)
+            assert message["data"] == data
+
+        message = {"id": 1, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+
+        manager = PerspectiveManager()
+        session = manager.new_session()
+        client_id = session.client_id
+        table = Table(data)
+
+        manager.host_table("table1", table)
+
+        # create a view through the session to make sure it has a client id
+        session.process(message, self.post)
+
+        # make sure the client ID is attached to the new view
+        assert len(manager._views.keys()) == 1
+        assert manager.get_view("view1")._client_id == client_id
+
+        to_dict_message = {"id": 2, "name": "view1", "cmd": "view_method", "method": "to_dict"}
+
+        session.process(to_dict_message, handle_to_dict)
+        assert s.get() is True
+
+    def test_session_multiple_new_sessions(self, sentinel):
+        s = sentinel(0)
+
+        def handle_to_dict(msg):
+            s.set(s.get() + 1)
+
+            message = json.loads(msg)
+
+            assert message["data"] == {
+                "a": [1, 2, 3, 1, 2, 3],
+                "b": ["a", "b", "c", "str1", "str2", "str3"]
+            }
+
+        manager = PerspectiveManager()
+        sessions = [manager.new_session() for i in range(5)]
+        table = Table(data)
+
+        manager.host_table("table1", table)
+
+        # create a view on each session
+        for i, session in enumerate(sessions):
+            # IDs have to conflict - each viewer will send the first message as
+            # ID = 1, so we need to make sure we handle that.
+            msg = {"id": 1, "table_name": "table1", "view_name": "view" + str(i), "cmd": "view"}
+            session.process(msg, self.post)
+
+        manager_views = list(manager._views.keys())
+        assert manager_views == ["view" + str(i) for i in range(5)]
+
+        for i in range(len(manager_views)):
+            view = manager.get_view(manager_views[i])
+            assert view._client_id == sessions[i].client_id
+
+        # arbitrarily do an update
+        random_session_id = random.randint(0, 4)
+        update_message = {"id": 2, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [1, 2, 3], "b": ["str1", "str2", "str3"]}]}
+        sessions[random_session_id].process(update_message, self.post)
+
+        # should reflect in all sessions
+        for i, session in enumerate(sessions):
+            to_dict_message = {"id": 3, "name": "view" + str(i), "cmd": "view_method", "method": "to_dict"}
+            session.process(to_dict_message, handle_to_dict)
+
+        assert s.get() == 5
+
+    def test_session_close_session_with_callbacks(self, sentinel):
+        s = sentinel(0)
+
+        manager = PerspectiveManager()
+        session = manager.new_session()
+        client_id = session.client_id
+
+        # create a table and view using manager
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        session.process(make_table, self.post)
+
+        make_view = {"id": 2, "table_name": "table1", "view_name": "view1", "cmd": "view"}
+        session.process(make_view, self.post)
+
+        # make sure the client ID is attached to the new view
+        assert len(manager._views.keys()) == 1
+        assert manager.get_view("view1")._client_id == client_id
+
+        def callback(updated):
+            assert updated["port_id"] == 0
+            s.set(s.get() + 100)
+
+        # simulate a client that holds callbacks by id
+        callbacks = {
+            3: callback
+        }
+
+        def post_update(msg):
+            # when `on_update` is triggered, this callback gets the message
+            # and has to decide which callback to trigger.
+            message = json.loads(msg)
+            assert message["id"] is not None
+            if message["id"] == 3:
+                # trigger callback
+                assert message["data"] == {
+                    "port_id": 0
+                }
+                callbacks[message["id"]](message["data"])
+
+        # hook into the created view and pass it the callback
+        make_on_update = {"id": 3, "name": "view1", "cmd": "view_method", "subscribe": True, "method": "on_update", "callback_id": "callback_1"}
+        session.process(make_on_update, post_update)
+
+        # call updates
+        update1 = {"id": 4, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [4], "b": ["d"]}]}
+        update2 = {"id": 5, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [5], "b": ["e"]}]}
+
+        session.process(update1, self.post)
+        session.process(update2, self.post)
+
+        assert s.get() == 200
+
+        # close the session
+        session.close()
+
+        # make sure the view is gone - but not the table
+        assert "table1" in manager._tables
+        assert manager._views == {}
+        assert len(manager._callback_cache) == 0
+
+    def test_session_close_multiple_sessions_with_callbacks(self, sentinel):
+        s = sentinel(0)
+
+        manager = PerspectiveManager()
+        sessions = [manager.new_session() for i in range(5)]
+
+        # create a table and view using manager
+        make_table = {"id": 1, "name": "table1", "cmd": "table", "args": [data]}
+        manager._process(make_table, self.post)
+
+        # create a view on each session
+        for i, session in enumerate(sessions):
+            # IDs have to conflict - each viewer will send the first message as
+            # ID = 1, so we need to make sure we handle that.
+            msg = {"id": 2, "table_name": "table1", "view_name": "view" + str(i), "cmd": "view"}
+            session.process(msg, self.post)
+
+        manager_views = list(manager._views.keys())
+        assert manager_views == ["view" + str(i) for i in range(5)]
+
+        for i in range(len(manager_views)):
+            view = manager.get_view(manager_views[i])
+            assert view._client_id == sessions[i].client_id
+
+        def callback(updated):
+            assert updated["port_id"] == 0
+            s.set(s.get() + 100)
+
+        # simulate a client that holds callbacks by id
+        callbacks = {
+            3: callback
+        }
+
+        def post_update(msg):
+            # when `on_update` is triggered, this callback gets the message
+            # and has to decide which callback to trigger.
+            message = json.loads(msg)
+            assert message["id"] is not None
+            if message["id"] == 3:
+                # trigger callback
+                assert message["data"] == {
+                    "port_id": 0
+                }
+                callbacks[message["id"]](message["data"])
+
+        # create a view and an on_update on each session
+        for i, session in enumerate(sessions):
+            view_name = "view" + str(i)
+            # IDs have to conflict - each viewer will send the first message as
+            # ID = 1, so we need to make sure we handle that.
+            msg = {"id": 2, "table_name": "table1", "view_name": view_name, "cmd": "view"}
+            session.process(msg, self.post)
+            make_on_update = {"id": 3, "name": view_name, "cmd": "view_method", "subscribe": True, "method": "on_update", "callback_id": "callback_1"}
+            session.process(make_on_update, post_update)
+
+        # call updates using a random session - they should propagate
+        random_session_id = random.randint(0, 4)
+        random_session = sessions[random_session_id]
+        random_client_id = random_session.client_id
+
+        update1 = {"id": 4, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [4], "b": ["d"]}]}
+        update2 = {"id": 5, "name": "table1", "cmd": "table_method", "method": "update", "args": [{"a": [5], "b": ["e"]}]}
+
+        random_session.process(update1, self.post)
+        random_session.process(update2, self.post)
+
+        # all updates processed, all callbacks fired
+        assert s.get() == 1000
+
+        # close a random session, and make sure the other views and callbacks
+        # are not affected
+        random_session.close()
+
+        # make sure the view is gone - but not the table
+        assert "table1" in manager._tables
+
+        assert "view" + str(random_session_id) not in manager._views.keys()
+        assert len(manager._views.keys()) == 4
+        
+        for callback in manager._callback_cache:
+            assert callback["client_id"] != random_client_id
+
+        assert len(manager._callback_cache) == 4
\ No newline at end of file
diff --git a/python/perspective/perspective/tests/table/test_view.py b/python/perspective/perspective/tests/table/test_view.py
index a3780a731d..367e85439f 100644
--- a/python/perspective/perspective/tests/table/test_view.py
+++ b/python/perspective/perspective/tests/table/test_view.py
@@ -753,7 +753,7 @@ def cb2(port_id):
         view.on_update(cb2)
         view.on_update(cb1)
         view.remove_update(cb1)
-        assert len(view._callbacks.get_callbacks()) == 1
+        assert len(view._callbacks) == 1
         tbl.update(data)
         assert s.get() == 2
 

From 96df86aa159625d16c5be62b6e212cd9945ab9ab Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Thu, 4 Jun 2020 16:14:04 -0400
Subject: [PATCH 3/8] make autocomplete results faster in UI for string filters

---
 .../src/js/viewer/dom_element.js                  | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/packages/perspective-viewer/src/js/viewer/dom_element.js b/packages/perspective-viewer/src/js/viewer/dom_element.js
index f8b2b900c7..6292923781 100644
--- a/packages/perspective-viewer/src/js/viewer/dom_element.js
+++ b/packages/perspective-viewer/src/js/viewer/dom_element.js
@@ -95,7 +95,8 @@ export class DomElement extends PerspectiveElement {
             row.setAttribute("filter", filter);
 
             if (type === "string") {
-                const view = this._table.view({row_pivots: [name], aggregates: {}});
+                // Get all unique values for the column
+                const view = this._table.view({row_pivots: [name], columns: []});
                 view.to_json().then(json => {
                     row.choices(this._autocomplete_choices(json));
                 });
@@ -517,9 +518,13 @@ export class DomElement extends PerspectiveElement {
     }
 
     _autocomplete_choices(json) {
-        return json
-            .slice(1, json.length)
-            .map(x => x.__ROW_PATH__)
-            .filter(x => (Array.isArray(x) ? x.filter(v => !!v).length > 0 : !!x));
+        const choices = [];
+        for (let i = 1; i < json.length; i++) {
+            const row_path = json[i].__ROW_PATH__;
+            if (Array.isArray(row_path) && row_path.length > 0) {
+                choices.push(row_path[0]);
+            }
+        }
+        return choices;
     }
 }

From a41a31cd445d54a8bca77952c1009683e7c69d9c Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Thu, 4 Jun 2020 17:20:01 -0400
Subject: [PATCH 4/8] fix tests

---
 .../src/js/viewer/dom_element.js               |  2 +-
 .../test/js/unit/dom_element.spec.js           |  3 ++-
 .../perspective/tests/manager/test_session.py  | 18 ++++++++++--------
 .../tests/table/test_table_arrow.py            |  2 +-
 .../tests/table/test_table_numpy.py            | 16 +++++++++++-----
 .../tests/table/test_table_pandas.py           |  8 ++++----
 .../perspective/tests/table/test_view.py       | 14 ++++++++++++++
 7 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/packages/perspective-viewer/src/js/viewer/dom_element.js b/packages/perspective-viewer/src/js/viewer/dom_element.js
index 6292923781..6d9988b060 100644
--- a/packages/perspective-viewer/src/js/viewer/dom_element.js
+++ b/packages/perspective-viewer/src/js/viewer/dom_element.js
@@ -521,7 +521,7 @@ export class DomElement extends PerspectiveElement {
         const choices = [];
         for (let i = 1; i < json.length; i++) {
             const row_path = json[i].__ROW_PATH__;
-            if (Array.isArray(row_path) && row_path.length > 0) {
+            if (Array.isArray(row_path) && row_path.length > 0 && row_path[0]) {
                 choices.push(row_path[0]);
             }
         }
diff --git a/packages/perspective-viewer/test/js/unit/dom_element.spec.js b/packages/perspective-viewer/test/js/unit/dom_element.spec.js
index 53d3b4ce12..adb1803241 100644
--- a/packages/perspective-viewer/test/js/unit/dom_element.spec.js
+++ b/packages/perspective-viewer/test/js/unit/dom_element.spec.js
@@ -17,13 +17,14 @@ describe(DomElement, () => {
             dom_element = new DomElement();
             json_choices = [
                 {__ROW_PATH__: [], foo: 2},
+                {__ROW_PATH__: [undefined], foo: 25},
                 {__ROW_PATH__: [null], foo: 25},
                 {__ROW_PATH__: ["somestring"], foo: 3},
                 {__ROW_PATH__: ["otherstring"], foo: 3}
             ];
         });
 
-        test("the first value and null values are filtered out", () => {
+        test("the first value, null values, and undefined values are filtered out", () => {
             expect(dom_element._autocomplete_choices(json_choices)).toEqual([["somestring"], ["otherstring"]]);
         });
     });
diff --git a/python/perspective/perspective/tests/manager/test_session.py b/python/perspective/perspective/tests/manager/test_session.py
index 623cdb95a0..3a161718af 100644
--- a/python/perspective/perspective/tests/manager/test_session.py
+++ b/python/perspective/perspective/tests/manager/test_session.py
@@ -83,11 +83,12 @@ def handle_to_dict(msg):
             session.process(msg, self.post)
 
         manager_views = list(manager._views.keys())
-        assert manager_views == ["view" + str(i) for i in range(5)]
+        for key in ["view" + str(i) for i in range(5)]:
+            assert key in manager_views
 
-        for i in range(len(manager_views)):
-            view = manager.get_view(manager_views[i])
-            assert view._client_id == sessions[i].client_id
+        for i, session in enumerate(sessions):
+            view = manager.get_view("view" + str(i))
+            assert view._client_id == session.client_id
 
         # arbitrarily do an update
         random_session_id = random.randint(0, 4)
@@ -179,11 +180,12 @@ def test_session_close_multiple_sessions_with_callbacks(self, sentinel):
             session.process(msg, self.post)
 
         manager_views = list(manager._views.keys())
-        assert manager_views == ["view" + str(i) for i in range(5)]
+        for key in ["view" + str(i) for i in range(5)]:
+            assert key in manager_views
 
-        for i in range(len(manager_views)):
-            view = manager.get_view(manager_views[i])
-            assert view._client_id == sessions[i].client_id
+        for i, session in enumerate(sessions):
+            view = manager.get_view("view" + str(i))
+            assert view._client_id == session.client_id
 
         def callback(updated):
             assert updated["port_id"] == 0
diff --git a/python/perspective/perspective/tests/table/test_table_arrow.py b/python/perspective/perspective/tests/table/test_table_arrow.py
index 271c4c32a7..e50c526a01 100644
--- a/python/perspective/perspective/tests/table/test_table_arrow.py
+++ b/python/perspective/perspective/tests/table/test_table_arrow.py
@@ -509,7 +509,7 @@ def test_table_arrow_loads_arrow_from_df_with_nan(self):
             "a": [1.5, 2.5, np.nan, 3.5, 4.5, np.nan, np.nan, np.nan]
         })
 
-        arrow_table = pa.Table.from_pandas(data)
+        arrow_table = pa.Table.from_pandas(data, preserve_index=False)
 
         assert arrow_table["a"].null_count == 4
 
diff --git a/python/perspective/perspective/tests/table/test_table_numpy.py b/python/perspective/perspective/tests/table/test_table_numpy.py
index e8c90d513a..ebb123b80a 100644
--- a/python/perspective/perspective/tests/table/test_table_numpy.py
+++ b/python/perspective/perspective/tests/table/test_table_numpy.py
@@ -237,28 +237,34 @@ def test_table_date(self):
         }
 
     def test_table_np_datetime(self):
-        data = {"a": np.array([datetime(2019, 7, 11, 12, 13)], dtype=np.datetime64), "b": np.array([datetime(2019, 7, 11, 12, 14)], dtype=np.datetime64)}
+        data = {"a": np.array([datetime(2019, 7, 11, 12, 13)], dtype="datetime64[ns]"), "b": np.array([datetime(2019, 7, 11, 12, 14)], dtype="datetime64[ns]")}
         tbl = Table(data)
         assert tbl.size() == 1
         assert tbl.schema() == {
             "a": datetime,
             "b": datetime
         }
-        assert tbl.view().to_numpy() == data
+        assert tbl.view().to_numpy() == {
+            "a": np.array([datetime(2019, 7, 11, 12, 13)], dtype=object),
+            "b": np.array([datetime(2019, 7, 11, 12, 14)], dtype=object)
+        }
 
     def test_table_np_datetime_mixed_dtype(self):
-        data = {"a": np.array([datetime(2019, 7, 11, 12, 13)], dtype=np.datetime64), "b": np.array([datetime(2019, 7, 11, 12, 14)], dtype=object)}
+        data = {"a": np.array([datetime(2019, 7, 11, 12, 13)], dtype="datetime64[ns]"), "b": np.array([datetime(2019, 7, 11, 12, 14)], dtype=object)}
         tbl = Table(data)
         assert tbl.size() == 1
         assert tbl.schema() == {
             "a": datetime,
             "b": datetime
         }
-        assert tbl.view().to_numpy() == data
+        assert tbl.view().to_numpy() == {
+            "a": np.array([datetime(2019, 7, 11, 12, 13)], dtype=object),
+            "b": np.array([datetime(2019, 7, 11, 12, 14)], dtype=object)
+        }
 
     def test_table_np_datetime_default(self):
         tbl = Table({
-            "a": np.array([datetime(2019, 7, 12, 11, 0)], dtype=np.datetime64)
+            "a": np.array([datetime(2019, 7, 12, 11, 0)], dtype="datetime64[ns]")
         })
 
         assert tbl.view().to_dict() == {
diff --git a/python/perspective/perspective/tests/table/test_table_pandas.py b/python/perspective/perspective/tests/table/test_table_pandas.py
index 50b7b907b0..0833ccc626 100644
--- a/python/perspective/perspective/tests/table/test_table_pandas.py
+++ b/python/perspective/perspective/tests/table/test_table_pandas.py
@@ -656,7 +656,7 @@ def test_table_pandas_update_datetime_schema_with_date(self):
     # Timestamps
 
     def test_table_pandas_timestamp_to_datetime(self):
-        data = [pd.Timestamp(2019, 7, 11, 12, 30, 5), None, pd.Timestamp(2019, 7, 11, 13, 30, 5), None]
+        data = [pd.Timestamp("2019-07-11 12:30:05"), None, pd.Timestamp("2019-07-11 13:30:05"), None]
         df = pd.DataFrame({
             "a": data
         })
@@ -664,7 +664,7 @@ def test_table_pandas_timestamp_to_datetime(self):
         assert table.view().to_dict()["a"] == [datetime(2019, 7, 11, 12, 30, 5), None, datetime(2019, 7, 11, 13, 30, 5), None]
 
     def test_table_pandas_timestamp_explicit_dtype(self):
-        data = [pd.Timestamp(2019, 7, 11, 12, 30, 5), None, pd.Timestamp(2019, 7, 11, 13, 30, 5), None]
+        data = [pd.Timestamp("2019-07-11 12:30:05"), None, pd.Timestamp("2019-07-11 13:30:05"), None]
         df = pd.DataFrame({
             "a": np.array(data, dtype="datetime64[ns]")
         })
@@ -672,12 +672,12 @@ def test_table_pandas_timestamp_explicit_dtype(self):
         assert table.view().to_dict()["a"] == [datetime(2019, 7, 11, 12, 30, 5), None, datetime(2019, 7, 11, 13, 30, 5), None]
 
     def test_table_pandas_update_datetime_with_timestamp(self):
-        data = [datetime(2019, 7, 11, 12, 30, 5), None, datetime(2019, 7, 11, 13, 30, 5), None]
+        data = [pd.Timestamp("2019-07-11 12:30:05"), None, pd.Timestamp("2019-07-11 13:30:05"), None]
         df = pd.DataFrame({
             "a": data
         })
         df2 = pd.DataFrame({
-            "a": [pd.Timestamp(2019, 7, 11, 12, 30, 5), None, pd.Timestamp(2019, 7, 11, 13, 30, 5), None]
+            "a": data
         })
         table = Table(df)
         table.update(df2)
diff --git a/python/perspective/perspective/tests/table/test_view.py b/python/perspective/perspective/tests/table/test_view.py
index 367e85439f..229b3e99eb 100644
--- a/python/perspective/perspective/tests/table/test_view.py
+++ b/python/perspective/perspective/tests/table/test_view.py
@@ -236,6 +236,20 @@ def test_view_no_columns(self):
         assert view.num_columns() == 0
         assert view.to_records() == [{}, {}]
 
+    def test_view_no_columns_pivoted(self):
+        data = [{"a": 1, "b": 2}, {"a": 3, "b": 4}]
+        tbl = Table(data)
+        view = tbl.view(row_pivots=["a"], columns=[])
+        assert view.num_columns() == 0
+        assert view.to_records() == [
+            {
+                "__ROW_PATH__": []
+            }, {
+                "__ROW_PATH__": ["1"]
+            }, {
+                "__ROW_PATH__": ["3"]
+            }]
+
     def test_view_specific_column(self):
         data = [{"a": 1, "b": 2, "c": 3, "d": 4}, {"a": 3, "b": 4, "c": 5, "d": 6}]
         tbl = Table(data)

From ef840d1c47bbe1d21c79dfeb6c709da81a8f3503 Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Thu, 4 Jun 2020 18:05:12 -0400
Subject: [PATCH 5/8] use pytz.utc instead of dateutil.utc in tests

---
 python/perspective/perspective/core/data/np.py          | 2 +-
 python/perspective/perspective/table/_date_validator.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/perspective/perspective/core/data/np.py b/python/perspective/perspective/core/data/np.py
index e888390c65..9883b59081 100644
--- a/python/perspective/perspective/core/data/np.py
+++ b/python/perspective/perspective/core/data/np.py
@@ -41,7 +41,7 @@ def make_null_mask(array):
 
         if not is_object_or_string_dtype:
             if is_datetime_dtype:
-                invalid = invalid or np.isnat(item)
+                invalid = invalid or str(item) == "NaT"
             else:
                 invalid = invalid or np.isnan(item)
 
diff --git a/python/perspective/perspective/table/_date_validator.py b/python/perspective/perspective/table/_date_validator.py
index 15b8bca776..f409b31b52 100644
--- a/python/perspective/perspective/table/_date_validator.py
+++ b/python/perspective/perspective/table/_date_validator.py
@@ -11,7 +11,7 @@
 from calendar import timegm
 from datetime import date, datetime
 from dateutil.parser import parse
-from dateutil.tz import UTC
+from pytz import UTC
 from pandas import Period
 from re import search
 from time import mktime

From f545f1fd48d018ee73662dc5759084b89033468f Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Fri, 5 Jun 2020 08:23:54 -0400
Subject: [PATCH 6/8] replace isinstance(str) with isinstance(six.string_types)
 for py2 compatibility

---
 .../perspective/manager/manager_internal.py    |  3 ++-
 .../perspective/perspective/table/_accessor.py |  6 +++---
 .../perspective/table/_date_validator.py       | 18 +++++++++---------
 python/perspective/perspective/table/table.py  |  5 +++--
 .../perspective/perspective/viewer/validate.py |  2 +-
 5 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/python/perspective/perspective/manager/manager_internal.py b/python/perspective/perspective/manager/manager_internal.py
index cef520933f..3c74635413 100644
--- a/python/perspective/perspective/manager/manager_internal.py
+++ b/python/perspective/perspective/manager/manager_internal.py
@@ -6,6 +6,7 @@
 # the Apache License 2.0.  The full license can be found in the LICENSE file.
 #
 
+from six import string_types
 import datetime
 import logging
 import json
@@ -56,7 +57,7 @@ def _process(self, msg, post_callback, client_id=None):
                 parameters: `data` (str), and `binary` (bool), a kwarg that
                 specifies whether `data` is a binary string.
         '''
-        if isinstance(msg, str):
+        if isinstance(msg, string_types):
             if msg == "heartbeat":   # TODO fix this
                 return
             msg = json.loads(msg)
diff --git a/python/perspective/perspective/table/_accessor.py b/python/perspective/perspective/table/_accessor.py
index 831d99b771..24bd7a16af 100644
--- a/python/perspective/perspective/table/_accessor.py
+++ b/python/perspective/perspective/table/_accessor.py
@@ -53,7 +53,7 @@ def _type_to_format(data_or_schema):
     elif isinstance(data_or_schema, dict):
         # schema or columns
         for v in data_or_schema.values():
-            if isinstance(v, type) or isinstance(v, str):
+            if isinstance(v, type) or isinstance(v, six.string_types):
                 # schema maps name -> type
                 return False, 2, list(data_or_schema.keys()), data_or_schema
             elif isinstance(v, list):
@@ -216,14 +216,14 @@ def marshal(self, cidx, ridx, dtype):
                     return str(val)
         elif dtype == t_dtype.DTYPE_DATE:
             # return datetime.date
-            if isinstance(val, str):
+            if isinstance(val, six.string_types):
                 parsed = self._date_validator.parse(val)
                 return self._date_validator.to_date_components(parsed)
             else:
                 return self._date_validator.to_date_components(val)
         elif dtype == t_dtype.DTYPE_TIME:
             # return unix timestamps for time
-            if isinstance(val, str):
+            if isinstance(val, six.string_types):
                 parsed = self._date_validator.parse(val)
                 return self._date_validator.to_timestamp(parsed)
             else:
diff --git a/python/perspective/perspective/table/_date_validator.py b/python/perspective/perspective/table/_date_validator.py
index f409b31b52..3a497c2eef 100644
--- a/python/perspective/perspective/table/_date_validator.py
+++ b/python/perspective/perspective/table/_date_validator.py
@@ -37,7 +37,7 @@ def _normalize_timestamp(obj):
 class _PerspectiveDateValidator(object):
     '''Validate and parse dates using the `dateutil` package.'''
 
-    def parse(self, str):
+    def parse(self, datestring):
         '''Return a datetime.datetime object containing the parsed date, or
         None if the date is invalid.
 
@@ -49,14 +49,14 @@ def parse(self, str):
         `timezone` property set.
 
         Args:
-            str (str): the datestring to parse
+            datestring (:obj:`str`): the datestring to parse
 
         Returns:
             (:class:`datetime.date`/`datetime.datetime`/`None`): if parse is
                 successful.
         '''
         try:
-            return parse(str)
+            return parse(datestring)
         except (ValueError, OverflowError):
             return None
 
@@ -180,7 +180,7 @@ def to_timestamp(self, obj):
         ms_timestamp = int(seconds_timestamp * 1000)
         return ms_timestamp
 
-    def format(self, s):
+    def format(self, datestring):
         '''Return either t_dtype.DTYPE_DATE or t_dtype.DTYPE_TIME depending on
         the format of the parsed date.
 
@@ -189,18 +189,18 @@ def format(self, s):
         to minimize false positives, i.e. do not parse dates without separators.
 
         Args:
-            str (str): the datestring to parse.
+            datestring (:obj:'str'): the datestring to parse.
         '''
-        if isinstance(s, (bytes, bytearray)):
-            s = s.decode("utf-8")
-        has_separators = bool(search(r"[/. -]", s))  # match commonly-used date separators
+        if isinstance(datestring, (bytes, bytearray)):
+            datestring = datestring.decode("utf-8")
+        has_separators = bool(search(r"[/. -]", datestring))  # match commonly-used date separators
         # match commonly-used date separators
 
         dtype = t_dtype.DTYPE_STR
 
         if has_separators:
             try:
-                parsed = parse(s)
+                parsed = parse(datestring)
                 if (parsed.hour, parsed.minute, parsed.second, parsed.microsecond) == (0, 0, 0, 0):
                     dtype = t_dtype.DTYPE_DATE
                 else:
diff --git a/python/perspective/perspective/table/table.py b/python/perspective/perspective/table/table.py
index 161f2c52b5..7460160b5b 100644
--- a/python/perspective/perspective/table/table.py
+++ b/python/perspective/perspective/table/table.py
@@ -6,6 +6,7 @@
 # the Apache License 2.0.  The full license can be found in the LICENSE file.
 #
 
+from six import string_types
 from datetime import date, datetime
 from .view import View
 from ._accessor import _PerspectiveAccessor
@@ -233,7 +234,7 @@ def is_valid_filter(self, filter):
         Returns:
             :obj:`bool`: Whether this filter is valid.
         '''
-        if isinstance(filter[1], str):
+        if isinstance(filter[1], string_types):
             filter_op = str_to_filter_op(filter[1])
         else:
             filter_op = filter[1]
@@ -251,7 +252,7 @@ def is_valid_filter(self, filter):
         schema = self.schema()
         in_schema = schema.get(filter[0], None)
         if in_schema and (schema[filter[0]] == date or schema[filter[0]] == datetime):
-            if isinstance(value, str):
+            if isinstance(value, string_types):
                 value = self._date_validator.parse(value)
 
         return value is not None
diff --git a/python/perspective/perspective/viewer/validate.py b/python/perspective/perspective/viewer/validate.py
index b992aa09bf..1ee85613d4 100644
--- a/python/perspective/perspective/viewer/validate.py
+++ b/python/perspective/perspective/viewer/validate.py
@@ -127,7 +127,7 @@ def validate_computed_columns(computed_columns):
 
     if isinstance(computed_columns, list):
         for c in computed_columns:
-            if not isinstance(c, dict) and not isinstance(c, str):
+            if not isinstance(c, dict) and not isinstance(c, string_types):
                 raise PerspectiveError('`computed_columns` kwarg must be a list of dicts or strings!')
             if isinstance(c, dict):
                 keys = c.keys()

From 8b9ee5d55bec79da423f4cb43cfce4e807168fba Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Fri, 5 Jun 2020 10:39:57 -0400
Subject: [PATCH 7/8] __ROW_PATH__ in python now returns python values and not
 strings

---
 .../perspective/include/perspective/python.h  |  1 +
 .../perspective/table/_data_formatter.py      |  6 +-
 .../perspective/tests/manager/test_manager.py |  4 +-
 .../tests/table/test_table_datetime.py        | 72 +++++++++----------
 .../perspective/tests/table/test_to_format.py | 70 +++++++++---------
 .../perspective/tests/table/test_view.py      | 41 +++++------
 .../tests/table/test_view_computed.py         |  4 +-
 7 files changed, 100 insertions(+), 98 deletions(-)

diff --git a/python/perspective/perspective/include/perspective/python.h b/python/perspective/perspective/include/perspective/python.h
index d36f5256f3..6c810539e7 100644
--- a/python/perspective/perspective/include/perspective/python.h
+++ b/python/perspective/perspective/include/perspective/python.h
@@ -382,6 +382,7 @@ PYBIND11_MODULE(libbinding, m)
     m.def("get_computation_input_types", &get_computation_input_types);
     m.def("get_computed_functions", &get_computed_functions);
     m.def("make_computations", &make_computations);
+    m.def("scalar_to_py", &scalar_to_py);
 }
 
 #endif
diff --git a/python/perspective/perspective/table/_data_formatter.py b/python/perspective/perspective/table/_data_formatter.py
index a2aed6de5e..4404cbd5df 100644
--- a/python/perspective/perspective/table/_data_formatter.py
+++ b/python/perspective/perspective/table/_data_formatter.py
@@ -11,7 +11,8 @@
 from ._constants import COLUMN_SEPARATOR_STRING
 from .libbinding import get_data_slice_zero, get_data_slice_one, get_data_slice_two, \
     get_from_data_slice_zero, get_from_data_slice_one, get_from_data_slice_two, \
-    get_pkeys_from_data_slice_zero, get_pkeys_from_data_slice_one, get_pkeys_from_data_slice_two
+    get_pkeys_from_data_slice_zero, get_pkeys_from_data_slice_one, \
+    get_pkeys_from_data_slice_two, scalar_to_py
 
 
 def _mod(a, b):
@@ -54,8 +55,7 @@ def to_format(options, view, output_format):
                 continue
             elif cidx == options["start_col"] and view._sides > 0:
                 if options["has_row_path"]:
-                    paths = [path.to_string(False) for path in row_path]
-                    paths.reverse()
+                    paths = [scalar_to_py(path, False, False) for path in reversed(row_path)]
                     if output_format == 'records':
                         data[-1]["__ROW_PATH__"] = paths
                     elif output_format in ('dict', 'numpy'):
diff --git a/python/perspective/perspective/tests/manager/test_manager.py b/python/perspective/perspective/tests/manager/test_manager.py
index 2d237bc97e..c2773fbbe2 100644
--- a/python/perspective/perspective/tests/manager/test_manager.py
+++ b/python/perspective/perspective/tests/manager/test_manager.py
@@ -178,7 +178,7 @@ def test_manager_create_view_one(self):
         manager.host_table("table1", table)
         manager._process(message, self.post)
         assert manager._views["view1"].to_dict() == {
-            "__ROW_PATH__": [[], ["1"], ["2"], ["3"]],
+            "__ROW_PATH__": [[], [1], [2], [3]],
             "a": [6, 1, 2, 3],
             "b": [3, 1, 1, 1]
         }
@@ -190,7 +190,7 @@ def test_manager_create_view_two(self):
         manager.host_table("table1", table)
         manager._process(message, self.post)
         assert manager._views["view1"].to_dict() == {
-            "__ROW_PATH__": [[], ["1"], ["2"], ["3"]],
+            "__ROW_PATH__": [[], [1], [2], [3]],
             "a|a": [1, 1, None, None],
             "a|b": [1, 1, None, None],
             "b|a": [2, None, 2, None],
diff --git a/python/perspective/perspective/tests/table/test_table_datetime.py b/python/perspective/perspective/tests/table/test_table_datetime.py
index d5d7347db1..4e1d8f6c6d 100644
--- a/python/perspective/perspective/tests/table/test_table_datetime.py
+++ b/python/perspective/perspective/tests/table/test_table_datetime.py
@@ -1042,9 +1042,9 @@ def test_table_row_pivot_datetime_row_path_local_time_EST(self):
             assert view.to_columns() == {
                 "__ROW_PATH__": [
                     [],
-                    ['2019-01-11 00:10:20.000 EST'],
-                    ['2019-01-11 11:10:20.000 EST'],
-                    ['2019-01-11 19:10:20.000 EST'],
+                    [datetime(2019, 1, 11, 0, 10, 20)],
+                    [datetime(2019, 1, 11, 11, 10, 20)],
+                    [datetime(2019, 1, 11, 19, 10, 20)],
                 ],
                 "a": [3, 1, 1, 1],
                 "b": [3, 0, 1, 2]
@@ -1071,9 +1071,9 @@ def test_table_row_pivot_datetime_row_path_UTC(self):
             assert view.to_columns() == {
                 "__ROW_PATH__": [
                     [],
-                    ['2019-01-11 00:10:20.000 UTC'],
-                    ['2019-01-11 11:10:20.000 UTC'],
-                    ['2019-01-11 19:10:20.000 UTC'],
+                    [datetime(2019, 1, 11, 0, 10, 20)],
+                    [datetime(2019, 1, 11, 11, 10, 20)],
+                    [datetime(2019, 1, 11, 19, 10, 20)],
                 ],
                 "a": [3, 1, 1, 1],
                 "b": [3, 0, 1, 2]
@@ -1096,9 +1096,9 @@ def test_table_row_pivot_datetime_row_path_CST(self):
             assert view.to_columns() == {
                 "__ROW_PATH__": [
                     [],
-                    ['2019-01-11 00:10:20.000 CST'],
-                    ['2019-01-11 11:10:20.000 CST'],
-                    ['2019-01-11 19:10:20.000 CST'],
+                    [datetime(2019, 1, 11, 0, 10, 20)],
+                    [datetime(2019, 1, 11, 11, 10, 20)],
+                    [datetime(2019, 1, 11, 19, 10, 20)],
                 ],
                 "a": [3, 1, 1, 1],
                 "b": [3, 0, 1, 2]
@@ -1121,9 +1121,9 @@ def test_table_row_pivot_datetime_row_path_PST(self):
             assert view.to_columns() == {
                 "__ROW_PATH__": [
                     [],
-                    ['2019-01-11 00:10:20.000 PST'],
-                    ['2019-01-11 11:10:20.000 PST'],
-                    ['2019-01-11 19:10:20.000 PST'],
+                    [datetime(2019, 1, 11, 0, 10, 20)],
+                    [datetime(2019, 1, 11, 11, 10, 20)],
+                    [datetime(2019, 1, 11, 19, 10, 20)],
                 ],
                 "a": [3, 1, 1, 1],
                 "b": [3, 0, 1, 2]
@@ -1323,18 +1323,18 @@ def test_table_row_pivot_date_correct(self):
         assert view.to_columns() == {
             "__ROW_PATH__": [
                 [],
-                ['2020-01-15'],
-                ['2020-02-15'],
-                ['2020-03-15'],
-                ['2020-04-15'],
-                ['2020-05-15'],
-                ['2020-06-15'],
-                ['2020-07-15'],
-                ['2020-08-15'],
-                ['2020-09-15'],
-                ['2020-10-15'],
-                ['2020-11-15'],
-                ['2020-12-15']
+                [datetime(2020, 1, 15, 0, 0)],
+                [datetime(2020, 2, 15, 0, 0)],
+                [datetime(2020, 3, 15, 0, 0)],
+                [datetime(2020, 4, 15, 0, 0)],
+                [datetime(2020, 5, 15, 0, 0)],
+                [datetime(2020, 6, 15, 0, 0)],
+                [datetime(2020, 7, 15, 0, 0)],
+                [datetime(2020, 8, 15, 0, 0)],
+                [datetime(2020, 9, 15, 0, 0)],
+                [datetime(2020, 10, 15, 0, 0)],
+                [datetime(2020, 11, 15, 0, 0)],
+                [datetime(2020, 12, 15, 0, 0)]
             ],
             "a": [12, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
             "b": [78, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
@@ -1350,18 +1350,18 @@ def test_table_row_pivot_pandas_date_correct(self):
         assert view.to_columns() == {
             "__ROW_PATH__": [
                 [],
-                ['2020-01-15'],
-                ['2020-02-15'],
-                ['2020-03-15'],
-                ['2020-04-15'],
-                ['2020-05-15'],
-                ['2020-06-15'],
-                ['2020-07-15'],
-                ['2020-08-15'],
-                ['2020-09-15'],
-                ['2020-10-15'],
-                ['2020-11-15'],
-                ['2020-12-15']
+                [datetime(2020, 1, 15, 0, 0)],
+                [datetime(2020, 2, 15, 0, 0)],
+                [datetime(2020, 3, 15, 0, 0)],
+                [datetime(2020, 4, 15, 0, 0)],
+                [datetime(2020, 5, 15, 0, 0)],
+                [datetime(2020, 6, 15, 0, 0)],
+                [datetime(2020, 7, 15, 0, 0)],
+                [datetime(2020, 8, 15, 0, 0)],
+                [datetime(2020, 9, 15, 0, 0)],
+                [datetime(2020, 10, 15, 0, 0)],
+                [datetime(2020, 11, 15, 0, 0)],
+                [datetime(2020, 12, 15, 0, 0)]
             ],
             "index": [66, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11],
             "a": [12, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
diff --git a/python/perspective/perspective/tests/table/test_to_format.py b/python/perspective/perspective/tests/table/test_to_format.py
index 42625069ec..0c14ffbb07 100644
--- a/python/perspective/perspective/tests/table/test_to_format.py
+++ b/python/perspective/perspective/tests/table/test_to_format.py
@@ -117,7 +117,7 @@ def test_to_records_one(self):
             row_pivots=["a"]
         )
         assert view.to_records() == [
-            {"__ROW_PATH__": [], "a": 2, "b": 2}, {"__ROW_PATH__": ["1"], "a": 2, "b": 2}
+            {"__ROW_PATH__": [], "a": 2, "b": 2}, {"__ROW_PATH__": [1], "a": 2, "b": 2}
         ]
 
     def test_to_records_two(self):
@@ -129,7 +129,7 @@ def test_to_records_two(self):
         )
         assert view.to_records() == [
             {"__ROW_PATH__": [], "string1|a": 1, "string1|b": 1, "string2|a": 1, "string2|b": 1},
-            {"__ROW_PATH__": ["1"], "string1|a": 1, "string1|b": 1, "string2|a": 1, "string2|b": 1},
+            {"__ROW_PATH__": [1], "string1|a": 1, "string1|b": 1, "string2|a": 1, "string2|b": 1},
         ]
 
     def test_to_records_column_only(self):
@@ -218,7 +218,7 @@ def test_to_dict_one(self):
             row_pivots=["a"]
         )
         assert view.to_dict() == {
-            "__ROW_PATH__": [[], ["1"]],
+            "__ROW_PATH__": [[], [1]],
             "a": [2, 2],
             "b": [4, 4]
         }
@@ -231,7 +231,7 @@ def test_to_dict_two(self):
             column_pivots=["b"]
         )
         assert view.to_dict() == {
-            "__ROW_PATH__": [[], ["1"]],
+            "__ROW_PATH__": [[], [1]],
             "2|a": [2, 2],
             "2|b": [4, 4]
         }
@@ -254,7 +254,7 @@ def test_to_dict_one_no_columns(self):
             row_pivots=["a"],
             columns=[]
         )
-        assert view.to_dict() == {"__ROW_PATH__": [[], ["1"]]}
+        assert view.to_dict() == {"__ROW_PATH__": [[], [1]]}
 
     def test_to_dict_two_no_columns(self):
         data = [{"a": 1, "b": 2}, {"a": 1, "b": 2}]
@@ -265,7 +265,7 @@ def test_to_dict_two_no_columns(self):
             columns=[]
         )
         assert view.to_dict() == {
-            "__ROW_PATH__": [[], ["1"]]
+            "__ROW_PATH__": [[], [1]]
         }
 
     def test_to_dict_column_only_no_columns(self):
@@ -343,7 +343,7 @@ def test_to_numpy_one(self):
             row_pivots=["a"]
         )
         v = view.to_numpy()
-        assert np.array_equal(v["__ROW_PATH__"], [[], ["1"]])
+        assert np.array_equal(v["__ROW_PATH__"], [[], [1]])
         assert np.array_equal(v["a"], np.array([2, 2]))
         assert np.array_equal(v["b"], np.array([4, 4]))
 
@@ -355,7 +355,7 @@ def test_to_numpy_two(self):
             column_pivots=["b"]
         )
         v = view.to_numpy()
-        assert np.array_equal(v["__ROW_PATH__"], [[], ["1"]])
+        assert np.array_equal(v["__ROW_PATH__"], [[], [1]])
         assert np.array_equal(v["2|a"], np.array([2, 2]))
         assert np.array_equal(v["2|b"], np.array([4, 4]))
 
@@ -421,8 +421,8 @@ def test_to_records_one_over_max_row(self):
         )
         assert records == [
             {'__ROW_PATH__': [], 'a': 5, 'b': 7},
-            {'__ROW_PATH__': ['1.5'], 'a': 1.5, 'b': 2.5},
-            {'__ROW_PATH__': ['3.5'], 'a': 3.5, 'b': 4.5}
+            {'__ROW_PATH__': [1.5], 'a': 1.5, 'b': 2.5},
+            {'__ROW_PATH__': [3.5], 'a': 3.5, 'b': 4.5}
         ]
 
     def test_to_records_two_over_max_row(self):
@@ -437,8 +437,8 @@ def test_to_records_two_over_max_row(self):
         )
         assert records == [
             {'2|a': 1, '2|b': 2, '4|a': 3, '4|b': 4, '__ROW_PATH__': []},
-            {'2|a': 1, '2|b': 2, '4|a': None, '4|b': None, '__ROW_PATH__': ['1']},
-            {'2|a': None, '2|b': None, '4|a': 3, '4|b': 4, '__ROW_PATH__': ['3']}
+            {'2|a': 1, '2|b': 2, '4|a': None, '4|b': None, '__ROW_PATH__': [1]},
+            {'2|a': None, '2|b': None, '4|a': 3, '4|b': 4, '__ROW_PATH__': [3]}
         ]
 
     def test_to_records_start_row(self):
@@ -501,8 +501,8 @@ def test_to_records_one_over_max_col(self):
         )
         assert records == [
             {'__ROW_PATH__': [], 'a': 5, 'b': 7},
-            {'__ROW_PATH__': ['1.5'], 'a': 1.5, 'b': 2.5},
-            {'__ROW_PATH__': ['3.5'], 'a': 3.5, 'b': 4.5}
+            {'__ROW_PATH__': [1.5], 'a': 1.5, 'b': 2.5},
+            {'__ROW_PATH__': [3.5], 'a': 3.5, 'b': 4.5}
         ]
 
     def test_to_records_two_over_max_col(self):
@@ -517,8 +517,8 @@ def test_to_records_two_over_max_col(self):
         )
         assert records == [
             {'2|a': 1, '2|b': 2, '4|a': 3, '4|b': 4, '__ROW_PATH__': []},
-            {'2|a': 1, '2|b': 2, '4|a': None, '4|b': None, '__ROW_PATH__': ['1']},
-            {'2|a': None, '2|b': None, '4|a': 3, '4|b': 4, '__ROW_PATH__': ['3']}
+            {'2|a': 1, '2|b': 2, '4|a': None, '4|b': None, '__ROW_PATH__': [1]},
+            {'2|a': None, '2|b': None, '4|a': 3, '4|b': 4, '__ROW_PATH__': [3]}
         ]
 
     def test_to_records_start_col(self):
@@ -552,8 +552,8 @@ def test_to_records_two_end_col(self):
         )
         assert records == [
             {'2|a': 1, '2|b': 2, '4|a': 3, '4|b': 4, '__ROW_PATH__': []},
-            {'2|a': 1, '2|b': 2, '4|a': None, '4|b': None, '__ROW_PATH__': ['1']},
-            {'2|a': None, '2|b': None, '4|a': 3, '4|b': 4, '__ROW_PATH__': ['3']}
+            {'2|a': 1, '2|b': 2, '4|a': None, '4|b': None, '__ROW_PATH__': [1]},
+            {'2|a': None, '2|b': None, '4|a': 3, '4|b': 4, '__ROW_PATH__': [3]}
         ]
 
     def test_to_records_start_col_end_col(self):
@@ -743,14 +743,14 @@ def test_to_csv_one(self):
         )
         if six.PY2:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__,a,b\r\n0,[],2,4\r\n1,[u'1'],2,4\r\n"
+                assert view.to_csv() == ",__ROW_PATH__,a,b\r\n0,[],2,4\r\n1,[1],2,4\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__,a,b\n0,[],2,4\n1,[u'1'],2,4\n"
+                assert view.to_csv() == ",__ROW_PATH__,a,b\n0,[],2,4\n1,[1],2,4\n"
         else:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__,a,b\r\n0,[],2,4\r\n1,['1'],2,4\r\n"
+                assert view.to_csv() == ",__ROW_PATH__,a,b\r\n0,[],2,4\r\n1,[1],2,4\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__,a,b\n0,[],2,4\n1,['1'],2,4\n"
+                assert view.to_csv() == ",__ROW_PATH__,a,b\n0,[],2,4\n1,[1],2,4\n"
 
     def test_to_csv_two(self):
         data = [{"a": 1, "b": 2}, {"a": 1, "b": 2}]
@@ -761,14 +761,14 @@ def test_to_csv_two(self):
         )
         if six.PY2:
             if IS_WIN:
-                assert view.to_csv() == ",2|a,2|b,__ROW_PATH__\r\n0,2,4,[]\r\n1,2,4,[u'1']\r\n"
+                assert view.to_csv() == ",2|a,2|b,__ROW_PATH__\r\n0,2,4,[]\r\n1,2,4,[1]\r\n"
             else:
-                assert view.to_csv() == ",2|a,2|b,__ROW_PATH__\n0,2,4,[]\n1,2,4,[u'1']\n"
+                assert view.to_csv() == ",2|a,2|b,__ROW_PATH__\n0,2,4,[]\n1,2,4,[1]\n"
         else:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__,2|a,2|b\r\n0,[],2,4\r\n1,['1'],2,4\r\n"
+                assert view.to_csv() == ",__ROW_PATH__,2|a,2|b\r\n0,[],2,4\r\n1,[1],2,4\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__,2|a,2|b\n0,[],2,4\n1,['1'],2,4\n"
+                assert view.to_csv() == ",__ROW_PATH__,2|a,2|b\n0,[],2,4\n1,[1],2,4\n"
 
     def test_to_csv_column_only(self):
         data = [{"a": 1, "b": 2}, {"a": 1, "b": 2}]
@@ -790,14 +790,14 @@ def test_to_csv_one_no_columns(self):
         )
         if six.PY2:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,[u'1']\r\n"
+                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,[1]\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,[u'1']\n"
+                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,[1]\n"
         else:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,['1']\r\n"
+                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,[1]\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,['1']\n"
+                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,[1]\n"
 
     def test_to_csv_two_no_columns(self):
         data = [{"a": 1, "b": 2}, {"a": 1, "b": 2}]
@@ -809,14 +809,14 @@ def test_to_csv_two_no_columns(self):
         )
         if six.PY2:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,[u'1']\r\n"
+                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,[1]\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,[u'1']\n"
+                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,[1]\n"
         else:
             if IS_WIN:
-                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,['1']\r\n"
+                assert view.to_csv() == ",__ROW_PATH__\r\n0,[]\r\n1,[1]\r\n"
             else:
-                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,['1']\n"
+                assert view.to_csv() == ",__ROW_PATH__\n0,[]\n1,[1]\n"
 
     def test_to_csv_column_only_no_columns(self):
         data = [{"a": 1, "b": 2}, {"a": 1, "b": 2}]
@@ -862,7 +862,7 @@ def test_to_format_implicit_index_two_dict(self):
             '4.5|a': [3.5, None, 3.5],
             '4.5|b': [4.5, None, 4.5],
             '__INDEX__': [[], [], []],  # index needs to be the same length as each column
-            '__ROW_PATH__': [[], ['1.5'], ['3.5']]
+            '__ROW_PATH__': [[], [1.5], [3.5]]
         }
 
     def test_to_format_implicit_index_np(self):
diff --git a/python/perspective/perspective/tests/table/test_view.py b/python/perspective/perspective/tests/table/test_view.py
index 229b3e99eb..af3a4103ed 100644
--- a/python/perspective/perspective/tests/table/test_view.py
+++ b/python/perspective/perspective/tests/table/test_view.py
@@ -43,8 +43,8 @@ def test_view_one(self):
         }
         assert view.to_records() == [
             {"__ROW_PATH__": [], "a": 4, "b": 6},
-            {"__ROW_PATH__": ["1"], "a": 1, "b": 2},
-            {"__ROW_PATH__": ["3"], "a": 3, "b": 4}
+            {"__ROW_PATH__": [1], "a": 1, "b": 2},
+            {"__ROW_PATH__": [3], "a": 3, "b": 4}
         ]
 
     def test_view_two(self):
@@ -59,8 +59,8 @@ def test_view_two(self):
         }
         assert view.to_records() == [
             {"2|a": 1, "2|b": 2, "4|a": 3, "4|b": 4, "__ROW_PATH__": []},
-            {"2|a": 1, "2|b": 2, "4|a": None, "4|b": None, "__ROW_PATH__": ["1"]},
-            {"2|a": None, "2|b": None, "4|a": 3, "4|b": 4, "__ROW_PATH__": ["3"]}
+            {"2|a": 1, "2|b": 2, "4|a": None, "4|b": None, "__ROW_PATH__": [1]},
+            {"2|a": None, "2|b": None, "4|a": 3, "4|b": 4, "__ROW_PATH__": [3]}
         ]
 
     def test_view_two_column_only(self):
@@ -245,9 +245,9 @@ def test_view_no_columns_pivoted(self):
             {
                 "__ROW_PATH__": []
             }, {
-                "__ROW_PATH__": ["1"]
+                "__ROW_PATH__": [1]
             }, {
-                "__ROW_PATH__": ["3"]
+                "__ROW_PATH__": [3]
             }]
 
     def test_view_specific_column(self):
@@ -316,8 +316,8 @@ def test_view_aggregates_with_no_columns(self):
         )
         assert view.to_records() == [
             {"__ROW_PATH__": []},
-            {"__ROW_PATH__": ["1"]},
-            {"__ROW_PATH__": ["3"]}
+            {"__ROW_PATH__": [1]},
+            {"__ROW_PATH__": [3]}
         ]
 
     def test_view_aggregates_column_order(self):
@@ -341,12 +341,11 @@ def test_view_row_pivot_datetime_row_paths_are_same_as_data(self):
         data = {"a": [datetime(2019, 7, 11, 12, 30)], "b": [1]}
         tbl = Table(data)
         view = tbl.view(row_pivots=["a"])
-
         data = view.to_dict()
 
         for rp in data["__ROW_PATH__"]:
             if len(rp) > 0:
-                assert rp[0] in ("2019-07-11 12:30:00.000 UTC", "2019-07-11 12:30:00.000 Coordinated Universal Time")
+                assert rp[0] == datetime(2019, 7, 11, 12, 30)
 
         assert tbl.view().to_dict() == {
             "a": [datetime(2019, 7, 11, 12, 30)], "b": [1]
@@ -378,8 +377,8 @@ def test_view_aggregate_int(self):
         )
         assert view.to_records() == [
             {"__ROW_PATH__": [], "a": 2.0, "b": 6},
-            {"__ROW_PATH__": ["1"], "a": 1.0, "b": 2},
-            {"__ROW_PATH__": ["3"], "a": 3.0, "b": 4}
+            {"__ROW_PATH__": [1], "a": 1.0, "b": 2},
+            {"__ROW_PATH__": [3], "a": 3.0, "b": 4}
         ]
 
     def test_view_aggregate_str(self):
@@ -402,9 +401,10 @@ def test_view_aggregate_datetime(self):
             aggregates={"a": "distinct count"},
             row_pivots=["a"]
         )
-        output = view.to_records()
-        row_path = output[1]["__ROW_PATH__"][0]
-        assert row_path in ("2019-10-01 11:30:00.000 UTC", "2019-10-01 11:30:00.000 Coordinated Universal Time")
+        assert view.to_records() == [
+            {"__ROW_PATH__": [], "a": 1},
+            {"__ROW_PATH__": [datetime(2019, 10, 1, 11, 30)], "a": 1}
+        ]
 
     def test_view_aggregate_datetime_leading_zeroes(self):
         data = [{"a": datetime(2019, 1, 1, 5, 5, 5)}, {"a": datetime(2019, 1, 1, 5, 5, 5)}]
@@ -413,9 +413,10 @@ def test_view_aggregate_datetime_leading_zeroes(self):
             aggregates={"a": "distinct count"},
             row_pivots=["a"]
         )
-        output = view.to_records()
-        row_path = output[1]["__ROW_PATH__"][0]
-        assert row_path in ("2019-01-01 05:05:05.000 UTC", "2019-01-01 05:05:05.000 Coordinated Universal Time")
+        assert view.to_records() == [
+            {"__ROW_PATH__": [], "a": 1},
+            {"__ROW_PATH__": [datetime(2019, 1, 1, 5, 5, 5)], "a": 1}
+        ]
 
     def test_view_aggregate_multiple_columns(self):
         data = [
@@ -804,7 +805,7 @@ def cb1(port_id, delta):
         tbl = Table(data)
         view = tbl.view(row_pivots=["a"])
         assert view.to_dict() == {
-            "__ROW_PATH__": [[], ["1"], ["3"]],
+            "__ROW_PATH__": [[], [1], [3]],
             "a": [4, 1, 3],
             "b": [6, 2, 4]
         }
@@ -831,7 +832,7 @@ def cb1(port_id, delta):
         tbl = Table(data)
         view = tbl.view(row_pivots=["a"], column_pivots=["b"])
         assert view.to_dict() == {
-            "__ROW_PATH__": [[], ["1"], ["3"]],
+            "__ROW_PATH__": [[], [1], [3]],
             "2|a": [1, 1, None],
             "2|b": [2, 2, None],
             "4|a": [3, None, 3],
diff --git a/python/perspective/perspective/tests/table/test_view_computed.py b/python/perspective/perspective/tests/table/test_view_computed.py
index cdcc07d8b3..0191905687 100644
--- a/python/perspective/perspective/tests/table/test_view_computed.py
+++ b/python/perspective/perspective/tests/table/test_view_computed.py
@@ -228,12 +228,12 @@ def test_view_computed_with_row_pivots(self):
             }]
         )
         assert view.to_columns() == {
-            "__ROW_PATH__": [[], ["6"], ["8"], ["10"], ["12"]],
+            "__ROW_PATH__": [[], [6], [8], [10], [12]],
             "a": [10, 1, 2, 3, 4],
             "b": [26, 5, 6, 7, 8],
             "computed": [36.0, 6.0, 8.0, 10.0, 12.0]
         }
-    
+
     def test_view_computed_with_column_pivots(self):
         table = Table({
             "a": [1, 2, 3, 4],

From 882030828ffe2a806711af9b7f26bec837f21775 Mon Sep 17 00:00:00 2001
From: Jun Tan <tanjunyuan@hotmail.co.uk>
Date: Tue, 16 Jun 2020 19:32:44 -0400
Subject: [PATCH 8/8] fix string filtering tests

---
 packages/perspective-viewer/test/js/unit/dom_element.spec.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/perspective-viewer/test/js/unit/dom_element.spec.js b/packages/perspective-viewer/test/js/unit/dom_element.spec.js
index adb1803241..9e78b88ed6 100644
--- a/packages/perspective-viewer/test/js/unit/dom_element.spec.js
+++ b/packages/perspective-viewer/test/js/unit/dom_element.spec.js
@@ -25,7 +25,7 @@ describe(DomElement, () => {
         });
 
         test("the first value, null values, and undefined values are filtered out", () => {
-            expect(dom_element._autocomplete_choices(json_choices)).toEqual([["somestring"], ["otherstring"]]);
+            expect(dom_element._autocomplete_choices(json_choices)).toEqual(["somestring", "otherstring"]);
         });
     });
 });