Merge pull request #6552 from VesnaT/pickle_ids

Table: Assure unique table.ids when unpickling

Orange/data/sql/table.py

@@ -1,6 +1,7 @@
 """
 Support for example tables wrapping data stored on a PostgreSQL server.
 """
+import contextlib
 import functools
 import logging
 import threading
@@ -669,3 +670,26 @@ def get_nan_frequency_attribute(self):
 
     def get_nan_frequency_class(self):
         return self.__get_nan_frequency(self.domain.class_vars)
+
+    def __getstate__(self):
+        # avoids locking magic in Table.__getstate__
+        return self.__dict__
+
+    def __setstate__(self, state):
+        # avoid locking magic in Table.__setstate__
+        self.__dict__.update(state)
+
+        # if X is defined then it was already downloaded
+        # thus ids exist to, rewrite them
+        if self._X is not None:
+            self._init_ids(self)
+
+    # pylint: disable=unused-argument
+    def _update_locks(self, *args, **kwargs):
+        # avoid locking inherited from Table
+        return
+
+    # pylint: disable=unused-argument
+    def unlocked(self, *parts):
+        # avoid locking inherited from Table
+        return contextlib.nullcontext()

Orange/data/table.py

@@ -553,6 +553,8 @@ def no_view(x):
                 setattr(self, "Y", no_view(state.pop("_Y")))  # state["_Y"] is a 2d array
             self.__dict__.update(state)
 
+        self._init_ids(self)
+
     def __getstate__(self):
         # Compatibility with pickles before table locking:
         # return the same state as before table lock
@@ -1007,9 +1009,11 @@ def from_list(cls, domain, rows, weights=None):
 
     @classmethod
     def _init_ids(cls, obj):
+        length = int(obj.X.shape[0])
         with cls._next_instance_lock:
-            obj.ids = np.array(range(cls._next_instance_id, cls._next_instance_id + obj.X.shape[0]))
-            cls._next_instance_id += obj.X.shape[0]
+            nid = cls._next_instance_id
+            cls._next_instance_id += length
+        obj.ids = np.arange(nid, nid + length, dtype=int)
 
     @classmethod
     def new_id(cls):

Orange/tests/sql/test_sql_table.py

@@ -752,6 +752,26 @@ def test_pickling_restores_connection_pool(self):
 
         self.assertEqual(iris[0], iris2[0])
 
+    @dbt.run_on(["postgres"])
+    def test_pickling_respects_downloaded_state(self):
+        iris = SqlTable(self.conn, self.iris, inspect_values=True)
+        iris2 = pickle.loads(pickle.dumps(iris))
+        # pylint: disable=protected-access
+        self.assertIsNone(iris._X)
+        self.assertIsNone(iris2._X)
+        self.assertIsNone(iris._ids)
+        self.assertIsNone(iris2._ids)
+
+        # trigger download into X, Y, metas
+        iris.X.shape[0]  # pylint: disable=pointless-statement
+        self.assertIsNotNone(iris._X)
+        self.assertIsNotNone(iris._ids)
+        iris2 = pickle.loads(pickle.dumps(iris))
+        self.assertIsNotNone(iris2._X)
+        self.assertIsNotNone(iris2._ids)
+        np.testing.assert_equal(iris.X, iris2.X)
+        self.assertEqual(len(set(iris.ids) | set(iris2.ids)), 300)
+
     @dbt.run_on(["postgres"])
     def test_list_tables_with_schema(self):
         with self.backend.execute_sql_query("DROP SCHEMA IF EXISTS orange_tests CASCADE") as cur:

Orange/tests/test_table.py

@@ -723,6 +723,16 @@ def test_save_pickle(self):
         finally:
             os.remove("iris.pickle")
 
+    def test_read_pickle_ids(self):
+        table = data.Table("iris")
+        try:
+            table.save("iris.pickle")
+            table1 = data.Table.from_file("iris.pickle")
+            table2 = data.Table.from_file("iris.pickle")
+            self.assertEqual(len(set(table1.ids) | set(table2.ids)), 300)
+        finally:
+            os.remove("iris.pickle")
+
     def test_from_numpy(self):
         a = np.arange(20, dtype="d").reshape((4, 5)).copy()
         m = np.arange(4, dtype="d").reshape((4, 1)).copy()

Orange/widgets/visualize/tests/test_owvenndiagram.py

@@ -3,7 +3,6 @@
 
 import unittest
 from unittest.mock import patch
-from copy import deepcopy
 
 import numpy as np
 
@@ -36,7 +35,7 @@ def _select_data(self):
 
     def test_rows_id(self):
         data = Table('zoo')
-        data1 = deepcopy(data)
+        data1 = data.copy()
         with data1.unlocked():
             data1[:, 1] = 1
         self.widget.rowwise = True