Goodbye Table.get_column(..., view=True), hello Table.set_column

Author: janezd

Orange/data/table.py

@@ -1436,7 +1436,7 @@ def add_column(self, variable, data, to_metas=None):
         domain = Domain(attrs, classes, metavars)
         new_table = self.transform(domain)
         with new_table.unlocked(new_table.metas if to_metas else new_table.X):
-            new_table.get_column(variable, view=True)[:] = data
+            new_table.set_column(variable, data)
         return new_table
 
     def is_sparse(self):
@@ -1565,7 +1565,7 @@ def shuffle(self):
         self.W = self.W[ind]
         self.ids = self.ids[ind]
 
-    @deprecated("Table.get_column(..., view=True)")
+    @deprecated("Table.get_column (or Table.set_column if you must)")
     def get_column_view(self, index: Union[Integral, Variable]) -> np.ndarray:
         """
         An obsolete function that was supposed to return a view with a column
@@ -1613,7 +1613,7 @@ def _get_column_view(self, index: Integral) -> np.ndarray:
         else:
             return self.metas[:, -1 - index]
 
-    def get_column(self, index, copy=False, view=False):
+    def get_column(self, index, copy=False):
         """
         Return a column with values of `index`.
 
@@ -1624,54 +1624,55 @@ def get_column(self, index, copy=False, view=False):
         Args:
             index (int or str or Variable): attribute
             copy (bool): if set to True, ensure the result is a copy, not a view
-            view (bool): if set to True, check that result is a view, not a copy
 
         Returns:
             column (np.array): data column
         """
-        if copy and view:
-            raise ValueError(
-                "incompatible arguments; `copy` and `view` cannot both be True")
-
         if isinstance(index, Variable) and index not in self.domain:
             if index.compute_value is None:
-                raise ValueError(
-                    f"variable {index.name} is not in domain")
-            if view:
-                raise ValueError(
-                    f"cannot create a view to derived variable {index.name}")
+                raise ValueError(f"variable {index.name} is not in domain")
             return compute_column(index.compute_value, self)
 
         mapper = None
-        is_primitive = isinstance(index, Variable) and index.is_primitive()
         if not isinstance(index, Integral):
             if isinstance(index, DiscreteVariable) \
                     and index.values != self.domain[index].values:
-                if view:
-                    raise ValueError(
-                        "cannot create a view to discrete variable "
-                        f"{index.name} with different encoding")
-                else:
-                    mapper = index.get_mapper_from(self.domain[index])
+                mapper = index.get_mapper_from(self.domain[index])
             index = self.domain.index(index)
 
         col = self._get_column_view(index)
         if sp.issparse(col):
-            if view:
-                raise ValueError("cannot create a view into sparse matrix")
             col = col.toarray().reshape(-1)
+        if col.dtype == object and self.domain[index].is_primitive():
+            col = col.astype(np.float64)
         if mapper is not None:
             col = mapper(col)
         if copy and col.base is not None:
             col = col.copy()
-        if is_primitive and col.dtype == object:
-            # TODO: This should fail if view=True. However, if a call wants a
-            # view, it may not need the cast (e.g. assigns values into column
-            # of metas).
-            # Either drop view argument OR don't cast if view.
-            col = col.astype(np.float64)
         return col
 
+    def set_column(self, index: Union[int, str, Variable], data):
+        """
+        Set the values in the given column do `data`.
+
+        This function may be useful, but try avoiding it.
+
+        Table (or the corresponding
+        part must be unlocked). If variable is discrete, its encoding must
+        match the variable in the domain.
+
+        Args:
+            index (int, str, Variable): index of a column
+            data (object): a single value or 1d array of length len(self)
+        """
+        if not isinstance(index, Integral):
+            if isinstance(index, DiscreteVariable) \
+                    and self.domain[index].values != index.values:
+                raise ValueError(f"cannot set data for variable {index.name} "
+                                 "with different encoding")
+            index = self.domain.index(index)
+        self._get_column_view(index)[:] = data
+
     def _filter_is_defined(self, columns=None, negate=False):
         # structure of function is obvious; pylint: disable=too-many-branches
         def _sp_anynan(a):

Orange/data/tests/test_table.py

@@ -6,7 +6,6 @@
 import numpy as np
 import scipy.sparse as sp
 
-import Orange
 from Orange.data import (
     ContinuousVariable, DiscreteVariable, StringVariable,
     Domain, Table, IsDefined, FilterContinuous, Values, FilterString,
@@ -533,10 +532,6 @@ def test_get_column_proper_view(self):
         np.testing.assert_equal(col, data.X[:, 0])
         self.assertIs(col.base, data.X)
 
-        col = data.get_column(y, view=True)
-        np.testing.assert_equal(col, data.X[:, 0])
-        self.assertIs(col.base, data.X)
-
         col = data.get_column(y, copy=True)
         np.testing.assert_equal(col, data.X[:, 0])
         self.assertIsNone(col.base)
@@ -552,31 +547,20 @@ def test_get_column_computed(self):
         np.testing.assert_equal(col2, [2, 4, 6])
         self.assertIsNone(col2.base)
 
-        self.assertRaises(ValueError, data.get_column, y2, view=True)
-
-    def test_get_column_wrong_arguments(self):
-        self.assertRaises(
-            ValueError, self.data.get_column, self.data.domain["y"],
-            copy=True, view=True)
-
     def test_get_column_discrete(self):
         data, d = self.data, self.data.domain["d"]
 
         col = data.get_column(d)
         np.testing.assert_equal(col, [0, 0, 1])
         self.assertIs(col.base, data.X)
 
-        col = data.get_column(d, view=True)
-        np.testing.assert_equal(col, [0, 0, 1])
-        self.assertIs(col.base, data.X)
-
         col = data.get_column(d, copy=True)
         np.testing.assert_equal(col, [0, 0, 1])
         self.assertIsNone(col.base)
 
         e = DiscreteVariable("d", values=("a", "b"))
         assert e == d
-        col = data.get_column(e, view=True)
+        col = data.get_column(e)
         np.testing.assert_equal(col, [0, 0, 1])
         self.assertIs(col.base, data.X)
 
@@ -590,10 +574,6 @@ def test_get_column_discrete(self):
         col = data.get_column(e)
         np.testing.assert_equal(col, [0, 0, 2])
 
-        e = DiscreteVariable("d", values=("a", "b", "c"))
-        assert e == d  # because that's how Variable mapping works
-        self.assertRaises(ValueError, data.get_column, e, view=True)
-
         with data.unlocked(data.X):
             data.X = sp.csr_matrix(data.X)
         e = DiscreteVariable("d", values=("a", "c", "b"))
@@ -612,19 +592,11 @@ def test_sparse(self):
         self.assertFalse(sp.issparse(col))
         np.testing.assert_equal(col, orig_y)
 
-        self.assertRaises(ValueError, data.get_column, y, view=True)
-
         col = data.get_column(y, copy=True)
         self.assertFalse(sp.issparse(col))
         np.testing.assert_equal(col, orig_y)
 
     def test_get_column_no_variable(self):
-        self.assertRaises(ValueError, self.data.get_column,
-                          ContinuousVariable("y3"), view=True)
-
-        self.assertRaises(ValueError, self.data.get_column,
-                          ContinuousVariable("y3"), copy=True)
-
         self.assertRaises(ValueError, self.data.get_column,
                           ContinuousVariable("y3"))
 
@@ -635,10 +607,6 @@ def test_index_by_int(self):
         np.testing.assert_equal(col, data.X[:, 0])
         self.assertIs(col.base, data.X)
 
-        col = data.get_column(0, view=True)
-        np.testing.assert_equal(col, data.X[:, 0])
-        self.assertIs(col.base, data.X)
-
         col = data.get_column(0, copy=True)
         np.testing.assert_equal(col, data.X[:, 0])
         self.assertIsNone(col.base)
@@ -658,10 +626,6 @@ def test_index_by_int(self):
         np.testing.assert_equal(col, data.X[:, 0])
         self.assertIs(col.base, data.X)
 
-        col = data.get_column("y", view=True)
-        np.testing.assert_equal(col, data.X[:, 0])
-        self.assertIs(col.base, data.X)
-
         col = data.get_column("y", copy=True)
         np.testing.assert_equal(col, data.X[:, 0])
         self.assertIsNone(col.base)

Orange/widgets/data/owneighbors.py

@@ -164,7 +164,7 @@ def _data_with_similarity(self, indices):
         distances = self.distances[indices]
         with neighbours.unlocked(neighbours.metas):
             if distances.size > 0:
-                neighbours.get_column(dist_var, view=1)[:] = distances
+                neighbours.set_column(dist_var, distances)
         return neighbours
 
 

Orange/widgets/data/tests/test_owrank.py

@@ -378,7 +378,7 @@ def test_scores_nan_sorting(self):
         """Check NaNs are sorted last"""
         data = self.iris.copy()
         with data.unlocked():
-            data.get_column('petal length', view=True)[:] = np.nan
+            data.set_column('petal length', np.nan)
         self.send_signal(self.widget.Inputs.data, data)
         self.wait_until_finished()
 

Orange/widgets/unsupervised/owhierarchicalclustering.py

@@ -770,7 +770,7 @@ def commit(self):
             domain = Orange.data.Domain(attrs, classes, metas + (clust_var,))
             data = items.transform(domain)
             with data.unlocked(data.metas):
-                data.get_column(clust_var, view=True)[:] = c
+                data.set_column(clust_var, c)
 
             if selected_indices:
                 selected_data = data[mask]

Orange/widgets/unsupervised/owkmeans.py

@@ -545,8 +545,8 @@ def send_data(self):
         new_domain = add_columns(domain, metas=[cluster_var, silhouette_var])
         new_table = self.data.transform(new_domain)
         with new_table.unlocked(new_table.metas):
-            new_table.get_column(cluster_var, view=True)[:] = clust_ids
-            new_table.get_column(silhouette_var, view=True)[:] = scores
+            new_table.set_column(cluster_var, clust_ids)
+            new_table.set_column(silhouette_var, scores)
 
         domain_attributes = set(domain.attributes)
         centroid_attributes = [

Orange/widgets/unsupervised/owlouvainclustering.py

@@ -387,7 +387,7 @@ def _send_data(self):
         new_domain = add_columns(domain, metas=[cluster_var])
         new_table = self.data.transform(new_domain)
         with new_table.unlocked(new_table.metas):
-            new_table.get_column(cluster_var, view=True)[:] = new_partition
+            new_table.set_column(cluster_var, new_partition)
 
         self.Outputs.annotated_data.send(new_table)
 

Orange/widgets/unsupervised/tests/test_owlouvain.py

@@ -65,7 +65,7 @@ def test_empty_dataset(self):
         meta_var = ContinuousVariable(name='meta_var')
         table = Table.from_domain(domain=Domain([], metas=[meta_var]), n_rows=5)
         with table.unlocked():
-            table.get_column(meta_var, view=True)[:] = meta
+            table.set_column(meta_var, meta)
 
         self.send_signal(self.widget.Inputs.data, table)
         self.commit_and_wait()