Fix partial updates in Python using dicts

packages/perspective/test/js/updates.js

@@ -1576,7 +1576,7 @@ module.exports = perspective => {
     });
 
     describe("implicit index", function() {
-        it("should apply single partial update on unindexed table using row id from '__INDEX__'", async function() {
+        it("should partial update on unindexed table", async function() {
             let table = perspective.table(data);
             table.update([
                 {
@@ -1595,6 +1595,73 @@ module.exports = perspective => {
             table.delete();
         });
 
+        it("should partial update on unindexed table, column dataset", async function() {
+            let table = perspective.table(data);
+            table.update({
+                __INDEX__: [2],
+                y: ["new_string"]
+            });
+
+            let view = table.view();
+            let result = await view.to_json();
+
+            // does not unset any values
+            expect(result).toEqual([
+                {x: 1, y: "a", z: true},
+                {x: 2, y: "b", z: false},
+                {x: 3, y: "new_string", z: true},
+                {x: 4, y: "d", z: false}
+            ]);
+            view.delete();
+            table.delete();
+        });
+
+        it("should partial update and unset on unindexed table", async function() {
+            let table = perspective.table(data);
+            table.update([
+                {
+                    __INDEX__: 2,
+                    y: "new_string",
+                    z: null
+                }
+            ]);
+
+            let view = table.view();
+            let result = await view.to_json();
+
+            // does not unset any values
+            expect(result).toEqual([
+                {x: 1, y: "a", z: true},
+                {x: 2, y: "b", z: false},
+                {x: 3, y: "new_string", z: null},
+                {x: 4, y: "d", z: false}
+            ]);
+            view.delete();
+            table.delete();
+        });
+
+        it("should partial update and unset on unindexed table, column dataset", async function() {
+            let table = perspective.table(data);
+            table.update({
+                __INDEX__: [0, 2],
+                y: [undefined, "new_string"],
+                z: [null, undefined]
+            });
+
+            let view = table.view();
+            let result = await view.to_json();
+
+            // does not unset any values
+            expect(result).toEqual([
+                {x: 1, y: "a", z: null},
+                {x: 2, y: "b", z: false},
+                {x: 3, y: "new_string", z: true},
+                {x: 4, y: "d", z: false}
+            ]);
+            view.delete();
+            table.delete();
+        });
+
         it("should apply single multi-column partial update on unindexed table using row id from '__INDEX__'", async function() {
             let table = perspective.table(data);
             table.update([

python/perspective/perspective/table/_accessor.py

@@ -328,21 +328,34 @@ def _get_numpy_column(self, name):
         return deconstruct_numpy(data, mask)
 
     def _has_column(self, ridx, name):
-        """Given a column name, validate that it is in the row.
+        """Given a row index and a column name, validate that the column exists
+        in the row.
 
         This allows differentiation between value is None (unset) and value not
-        in row (no-op).
+        in row (no-op), which is important to prevent unintentional overwriting
+        of values during a partial update.
 
         Args:
-            ridx (int)
-            name (str)
+            ridx (:obj:`int`)
+            name (:obj:`str`)
 
         Returns:
             bool: True if column is in row, or if column belongs to pkey/op
                 columns required by the engine. False otherwise.
         """
-        if self._format != 0 or name in ("psp_pkey", "psp_okey", "psp_op"):
-            # no partial updates available on meta column, schema, dict updates
+        if self._format == 2 or name in ("psp_pkey", "psp_okey", "psp_op"):
+            # Schemas and reserved column names are always present.
             return True
+        elif self._format == 1:
+            # For dicts of lists, check whether the column is set in the dict
+            # itself. Because there is no way to specify an `undefined` value
+            # in Python, whether the column exists in the dict is enough for
+            # us to determine whether to write the column or not.
+            return name in self._data_or_schema
         else:
+            # For row-oriented datasets, check whether the specified row
+            # contains the column. This is important for datasets where
+            # a column might not be set at a given row, such as:
+            # [{a: 1, b: 2, c: 3}, {a: 2}], where we do not want to set values
+            # for columns "b" and "c" at row 1.
             return name in self._data_or_schema[ridx]

python/perspective/perspective/tests/table/test_update.py

@@ -37,7 +37,66 @@ def test_update_partial(self):
         tbl.update([{"a": "abc", "b": 456}])
         assert tbl.view().to_records() == [{"a": "abc", "b": 456}]
 
-    def test_update_partial_cols_not_in_schema(self):
+    def test_update_partial_add_row(self):
+        tbl = Table([{"a": "abc", "b": 123}], index="a")
+        tbl.update([{"a": "abc", "b": 456}, {"a": "def"}])
+        assert tbl.view().to_records() == [{"a": "abc", "b": 456}, {"a": "def", "b": None}]
+
+    def test_update_partial_noop(self):
+        tbl = Table([{"a": "abc", "b": 1, "c": 2}], index="a")
+        tbl.update([{"a": "abc", "b": 456}])
+        assert tbl.view().to_records() == [{"a": "abc", "b": 456, "c": 2}]
+
+    def test_update_partial_unset(self):
+        tbl = Table([{"a": "abc", "b": 1, "c": 2}, {"a": "def", "b": 3, "c": 4}], index="a")
+        tbl.update([{"a": "abc"}, {"a": "def", "c": None}])
+        assert tbl.view().to_records() == [{"a": "abc", "b": 1, "c": 2}, {"a": "def", "b": 3, "c": None}]
+
+    def test_update_columnar_partial_add_row(self):
+        tbl = Table([{"a": "abc", "b": 123}], index="a")
+
+        tbl.update({
+            "a": ["abc", "def"],
+            "b": [456, None]
+        })
+
+        assert tbl.view().to_records() == [{"a": "abc", "b": 456}, {"a": "def", "b": None}]
+
+    def test_update_columnar_partial_noop(self):
+        tbl = Table([{"a": "abc", "b": 1, "c": 2}], index="a")
+
+        # no-op because "c" is not in the update dataset
+        tbl.update({
+            "a": ["abc"],
+            "b": [456]
+        })
+
+        assert tbl.view().to_records() == [{"a": "abc", "b": 456, "c": 2}]
+
+    def test_update_columnar_partial_unset(self):
+        tbl = Table([{"a": "abc", "b": 1, "c": 2}, {"a": "def", "b": 3, "c": 4}], index="a")
+
+        tbl.update({
+            "a": ["abc"],
+            "b": [None]
+        })
+
+        assert tbl.view().to_records() == [{"a": "abc", "b": None, "c": 2}, {"a": "def", "b": 3, "c": 4}]
+
+    def test_update_partial_subcolumn(self):
+        tbl = Table([{"a": "abc", "b": 123, "c": 456}], index="a")
+        tbl.update([{"a": "abc", "c": 1234}])
+        assert tbl.view().to_records() == [{"a": "abc", "b": 123, "c": 1234}]
+
+    def test_update_partial_subcolumn_dict(self):
+        tbl = Table([{"a": "abc", "b": 123, "c": 456}], index="a")
+        tbl.update({
+            "a": ["abc"],
+            "c": [1234]
+        })
+        assert tbl.view().to_records() == [{"a": "abc", "b": 123, "c": 1234}]
+
+    def test_update_partial_cols_more_columns_than_table(self):
         tbl = Table([{"a": "abc", "b": 123}], index="a")
         tbl.update([{"a": "abc", "b": 456, "c": 789}])
         assert tbl.view().to_records() == [{"a": "abc", "b": 456}]
@@ -306,13 +365,28 @@ def test_update_implicit_index(self):
         }])
         assert view.to_records() == [{"a": 3, "b": 15}, {"a": 2, "b": 3}]
 
-    def test_update_implicit_index_dict_should_unset(self):
+    def test_update_implicit_index_dict_noop(self):
+        data = [{"a": 1, "b": 2}, {"a": 2, "b": 3}]
+        tbl = Table(data)
+        view = tbl.view()
+
+        # "b" does not exist in dataset, so no-op
+        tbl.update({
+            "__INDEX__": [0],
+            "a": [3],
+        })
+        assert view.to_records() == [{"a": 3, "b": 2}, {"a": 2, "b": 3}]
+
+    def test_update_implicit_index_dict_unset_with_null(self):
         data = [{"a": 1, "b": 2}, {"a": 2, "b": 3}]
         tbl = Table(data)
         view = tbl.view()
+
+        # unset because "b" is null
         tbl.update({
             "__INDEX__": [0],
-            "a": [3]
+            "a": [3],
+            "b": [None]
         })
         assert view.to_records() == [{"a": 3, "b": None}, {"a": 2, "b": 3}]
 

python/perspective/perspective/tests/table/test_update_arrow.py

@@ -95,16 +95,21 @@ def test_update_arrow_updates_dict_file(self):
     @mark.skip
     def test_update_arrow_updates_dict_partial_file(self):
         tbl = None
+        v = None
 
         with open(DICT_ARROW, mode='rb') as src:
             tbl = Table(src.read(), index="a")
-            tbl.update(src.read())
-            assert tbl.size() == 2
+            v = tbl.view()
+            assert v.num_rows() == 2
+            assert v.to_dict() == {
+                "a": ["abc", "def"],
+                "b": ["klm", "hij"]
+            }
 
         with open(DICT_UPDATE_ARROW, mode='rb') as partial:
             tbl.update(partial.read())
-            assert tbl.size() == 4
-            assert tbl.view().to_dict() == {
+            v.num_rows() == 4
+            assert v.to_dict() == {
                 "a": ["abc", "def", "update1", "update2"],
                 "b": ["klm", "hij", None, "update4"]
             }