Cautious delete bugfixes (LorenFrankLab#821)

* LorenFrankLab#820

* Update changelog

* Blackify

* logger.error -> TypeError

* LorenFrankLab#818

CHANGELOG.md

@@ -12,6 +12,8 @@
     - Initial implementation. #711, #762
     - More robust caching of join to downstream tables. #806
     - Overwrite datajoint `delete` method to use `cautious_delete`. #806
+    - Reverse join order for session summary. #821
+    - Add temporary logging of use to `common_usage`. #811, #821
 - Add `deprecation_factory` to facilitate table migration. #717
 - Add Spyglass logger. #730
 - IntervalList: Add secondary key `pipeline` #742

src/spyglass/utils/dj_chains.py

@@ -134,13 +134,12 @@ def __init__(self, parent: Table, child: Table, connection=None):
             get_master(child.full_table_name) == ""
             and MERGE_PK in child.heading.names
         ):
-            logger.error("Child is a merge table. Use TableChains instead.")
+            raise TypeError("Child is a merge table. Use TableChains instead.")
 
         self._link_symbol = " -> "
         self.parent = parent
         self.child = child
         self._has_link = child.full_table_name in parent.descendants()
-        self._errors = []
 
     def __str__(self):
         """Return string representation of chain: parent -> child."""
@@ -221,17 +220,26 @@ def objects(self) -> List[dj.FreeTable]:
             else None
         )
 
-    def errors(self) -> List[str]:
-        """Return list of errors for each table in chain."""
-        return self._errors
-
-    def join(self, restricton: str = None) -> dj.expression.QueryExpression:
-        """Return join of tables in chain with restriction applied to parent."""
+    def join(
+        self, restricton: str = None, reverse_order: bool = False
+    ) -> dj.expression.QueryExpression:
+        """Return join of tables in chain with restriction applied to parent.
+
+        Parameters
+        ----------
+        restriction : str, optional
+            Restriction to apply to first table in the order.
+            Defaults to self.parent.restriction.
+        reverse_order : bool, optional
+            If True, join tables in reverse order. Defaults to False.
+        """
         if not self._has_link:
             return None
+
+        objects = self.objects[::-1] if reverse_order else self.objects
         restriction = restricton or self.parent.restriction or True
-        join = self.objects[0] & restriction
-        for table in self.objects[1:]:
+        join = objects[0] & restriction
+        for table in objects[1:]:
             try:
                 join = join.proj() * table
             except dj.DataJointError as e:

src/spyglass/utils/dj_mixin.py

@@ -278,7 +278,9 @@ def _get_exp_summary(self):
         empty_pk = {self._member_pk: "NULL"}
 
         format = dj.U(self._session_pk, self._member_pk)
-        sess_link = self._session_connection.join(self.restriction)
+        sess_link = self._session_connection.join(
+            self.restriction, reverse_order=True
+        )
 
         exp_missing = format & (sess_link - SesExp).proj(**empty_pk)
         exp_present = format & (sess_link * SesExp - exp_missing).proj()
@@ -363,17 +365,29 @@ def _usage_table(self):
 
     def _log_use(self, start, merge_deletes=None):
         """Log use of cautious_delete."""
-        self._usage_table.insert1(
-            dict(
-                duration=time() - start,
-                dj_user=dj.config["database.user"],
-                origin=self.full_table_name,
-                restriction=(
-                    str(self.restriction)[:255] if self.restriction else "None"
-                ),
-                merge_deletes=merge_deletes,
-            )
+        if isinstance(merge_deletes, QueryExpression):
+            merge_deletes = merge_deletes.fetch(as_dict=True)
+        safe_insert = dict(
+            duration=time() - start,
+            dj_user=dj.config["database.user"],
+            origin=self.full_table_name,
         )
+        try:
+            self._usage_table.insert1(
+                dict(
+                    **safe_insert,
+                    restriction=(
+                        "".join(self.restriction)[255:]  # handle list
+                        if self.restriction
+                        else "None"
+                    ),
+                    merge_deletes=merge_deletes,
+                )
+            )
+        except (DataJointError, DataError):
+            self._usage_table.insert1(
+                dict(**safe_insert, restriction="Unknown")
+            )
 
     # TODO: Intercept datajoint delete confirmation prompt for merge deletes
     def cautious_delete(self, force_permission: bool = False, *args, **kwargs):