Fault-permit insert and remove mutual exclusivity protections on Merge (#824)

* #719, #804, #212

* #768

* Add merge delete and populate

* Changes following PR review @edeno

* Replace delayed import of ImportedSpikeSorting

Author: CBroz1

CHANGELOG.md

@@ -4,27 +4,38 @@
 
 ### Infrastructure
 
-- Additional documentation. #690
-- Clean up following pre-commit checks. #688
-- Add Mixin class to centralize `fetch_nwb` functionality. #692, #734
-- Refactor restriction use in `delete_downstream_merge` #703
-- Add `cautious_delete` to Mixin class
-    - 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
-- Increase pytest coverage for `common`, `lfp`, and `utils`. #743
-- Update docs to reflect new notebooks. #776
-- Add overview of Spyglass to docs. #779
-- Update linting for Black 24. #808
-- Steamline dependency management. #822
+- Docs:
+    - Additional documentation. #690
+    - Add overview of Spyglass to docs. #779
+    - Update docs to reflect new notebooks. #776
+- Mixin:
+    - Add Mixin class to centralize `fetch_nwb` functionality. #692, #734
+    - Refactor restriction use in `delete_downstream_merge` #703
+    - Add `cautious_delete` to Mixin class
+        - 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
+- Merge Tables:
+    - UUIDs: Revise Merge table uuid generation to include source. #824
+    - UUIDs: Remove mutual exclusivity logic due to new UUID generation. #824
+    - Add method for `merge_populate`. #824
+- Linting:
+    - Clean up following pre-commit checks. #688
+    - Update linting for Black 24. #808
+- Misc:
+    - Add `deprecation_factory` to facilitate table migration. #717
+    - Add Spyglass logger. #730
+    - Increase pytest coverage for `common`, `lfp`, and `utils`. #743
+    - Steamline dependency management. #822
 
 ### Pipelines
 
+- Common:
+    - `IntervalList`: Add secondary key `pipeline` #742
+    - Add `common_usage` table. #811, #821, #824
+    - Add catch errors during `populate_all_common`. #824
 - Spike sorting:
     - Add SpikeSorting V1 pipeline. #651
     - Move modules into spikesorting.v0 #807

src/spyglass/common/common_behav.py

@@ -42,6 +42,25 @@ class SpatialSeries(SpyglassMixin, dj.Part):
         name=null: varchar(32)       # name of spatial series
         """
 
+    def populate(self, keys=None):
+        """Insert position source data from NWB file.
+
+        WARNING: populate method on Manual table is not protected by transaction
+                protections like other DataJoint tables.
+        """
+        if not isinstance(keys, list):
+            keys = [keys]
+        if isinstance(keys[0], dj.Table):
+            keys = [k for tbl in keys for k in tbl.fetch("KEY", as_dict=True)]
+        for key in keys:
+            nwb_file_name = key.get("nwb_file_name")
+            if not nwb_file_name:
+                raise ValueError(
+                    "PositionSource.populate is an alias for a non-computed table "
+                    + "and must be passed a key with nwb_file_name"
+                )
+            self.insert_from_nwbfile(nwb_file_name)
+
     @classmethod
     def insert_from_nwbfile(cls, nwb_file_name):
         """Add intervals to ItervalList and PositionSource.
@@ -482,6 +501,7 @@ def _no_transaction_make(self, key):
 
         # Skip populating if no pos interval list names
         if len(pos_intervals) == 0:
+            # TODO: Now that populate_all accept errors, raise here?
             logger.error(f"NO POS INTERVALS FOR {key}; {no_pop_msg}")
             return
 
@@ -519,6 +539,7 @@ def _no_transaction_make(self, key):
 
         # Check that each pos interval was matched to only one epoch
         if len(matching_pos_intervals) != 1:
+            # TODO: Now that populate_all accept errors, raise here?
             logger.error(
                 f"Found {len(matching_pos_intervals)} pos intervals for {key}; "
                 + f"{no_pop_msg}\n{matching_pos_intervals}"

src/spyglass/common/common_usage.py

@@ -1,6 +1,7 @@
 """A schema to store the usage of advanced Spyglass features.
 
-Records show usage of features such as table chains, which will be used to
+Records show usage of features such as cautious delete and fault-permitting
+insert, which will be used to
 determine which features are used, how often, and by whom. This will help
 plan future development of Spyglass.
 """
@@ -21,3 +22,18 @@ class CautiousDelete(dj.Manual):
     restriction: varchar(255)
     merge_deletes = null: blob
     """
+
+
+@schema
+class InsertError(dj.Manual):
+    definition = """
+    id: int auto_increment
+    ---
+    dj_user: varchar(64)
+    connection_id: int     # MySQL CONNECTION_ID()
+    nwb_file_name: varchar(64)
+    table: varchar(64)
+    error_type: varchar(64)
+    error_message: varchar(255)
+    error_raw = null: blob
+    """

src/spyglass/common/populate_all_common.py

@@ -1,3 +1,5 @@
+import datajoint as dj
+
 from spyglass.common.common_behav import (
     PositionSource,
     RawPosition,
@@ -14,51 +16,58 @@
 from spyglass.common.common_nwbfile import Nwbfile
 from spyglass.common.common_session import Session
 from spyglass.common.common_task import TaskEpoch
+from spyglass.common.common_usage import InsertError
 from spyglass.utils import logger
 
 
 def populate_all_common(nwb_file_name):
-    # Insert session one by one
-    fp = [(Nwbfile & {"nwb_file_name": nwb_file_name}).proj()]
-    logger.info("Populate Session...")
-    Session.populate(fp)
-
-    # If we use Kachery for data sharing we can uncomment the following two lines. TBD
-    # logger.info('Populate NwbfileKachery...')
-    # NwbfileKachery.populate()
-
-    logger.info("Populate ElectrodeGroup...")
-    ElectrodeGroup.populate(fp)
-
-    logger.info("Populate Electrode...")
-    Electrode.populate(fp)
-
-    logger.info("Populate Raw...")
-    Raw.populate(fp)
-
-    logger.info("Populate SampleCount...")
-    SampleCount.populate(fp)
-
-    logger.info("Populate DIOEvents...")
-    DIOEvents.populate(fp)
-
-    # sensor data (from analog ProcessingModule) is temporarily removed from NWBFile
-    # to reduce file size while it is not being used. add it back in by commenting out
-    # the removal code in spyglass/data_import/insert_sessions.py when ready
-    # logger.info('Populate SensorData')
-    # SensorData.populate(fp)
-
-    logger.info("Populate TaskEpochs")
-    TaskEpoch.populate(fp)
-    logger.info("Populate StateScriptFile")
-    StateScriptFile.populate(fp)
-    logger.info("Populate VideoFile")
-    VideoFile.populate(fp)
-    logger.info("RawPosition...")
-    PositionSource.insert_from_nwbfile(nwb_file_name)
-    RawPosition.populate(fp)
-
-    logger.info("Populate ImportedSpikeSorting...")
+    """Insert all common tables for a given NWB file."""
     from spyglass.spikesorting.imported import ImportedSpikeSorting
 
-    ImportedSpikeSorting.populate(fp)
+    key = [(Nwbfile & f"nwb_file_name LIKE '{nwb_file_name}'").proj()]
+    tables = [
+        Session,
+        # NwbfileKachery, # Not used by default
+        ElectrodeGroup,
+        Electrode,
+        Raw,
+        SampleCount,
+        DIOEvents,
+        # SensorData, # Not used by default. Generates large files
+        RawPosition,
+        TaskEpoch,
+        StateScriptFile,
+        VideoFile,
+        PositionSource,
+        RawPosition,
+        ImportedSpikeSorting,
+    ]
+    error_constants = dict(
+        dj_user=dj.config["database.user"],
+        connection_id=dj.conn().connection_id,
+        nwb_file_name=nwb_file_name,
+    )
+
+    for table in tables:
+        logger.info(f"Populating {table.__name__}...")
+        try:
+            table.populate(key)
+        except Exception as e:
+            InsertError.insert1(
+                dict(
+                    **error_constants,
+                    table=table.__name__,
+                    error_type=type(e).__name__,
+                    error_message=str(e),
+                    error_raw=str(e),
+                )
+            )
+    query = InsertError & error_constants
+    if query:
+        err_tables = query.fetch("table")
+        logger.error(
+            f"Errors occurred during population for {nwb_file_name}:\n\t"
+            + f"Failed tables {err_tables}\n\t"
+            + "See common_usage.InsertError for more details"
+        )
+        return query.fetch("KEY")

src/spyglass/linearization/__init__.py

@@ -1 +1,3 @@
-from spyglass.linearization.merge import LinearizedPositionOutput
+# CB: Circular import if only importing PositionOutput
+
+# from spyglass.linearization.merge import LinearizedPositionOutput