Tidy position

.gitignore

@@ -137,6 +137,7 @@ dmypy.json
 *.videoTimeStamps
 *.cameraHWSync
 *.stateScriptLog
+tests/_data/*
 
 *.nwb
 *.DS_Store

CHANGELOG.md

@@ -6,6 +6,12 @@
 
 <!-- Running draft to be removed immediately prior to release. -->
 
+```python
+from spyglass.common.common_behav import PositionIntervalMap
+
+PositionIntervalMap.alter()
+```
+
 ### Infrastructure
 
 - Create class `SpyglassGroupPart` to aid delete propagations #899
@@ -19,7 +25,8 @@
 - Add pytests for position pipeline, various `test_mode` exceptions #966
 - Migrate `pip` dependencies from `environment.yml`s to `pyproject.toml` #966
 - Add documentation for common error messages #997
-- Allow mixin tables with parallelization in `make` to run populate with `processes > 1` #1001
+- Allow mixin tables with parallelization in `make` to run populate with
+    `processes > 1` #1001
 
 ### Pipelines
 
@@ -30,19 +37,28 @@
     - Remove unused `ElectrodeBrainRegion` table #1003
     - Files created by `AnalysisNwbfile.create()` receive new object_id #999,
         #1004
+    - Remove redundant calls to tables in `populate_all_common` #870
+    - Improve logging clarity in `populate_all_common` #870
+    - `PositionIntervalMap` now inserts null entries for missing intervals #870
 - Decoding: Default values for classes on `ImportError` #966
 - Position
     - Allow dlc without pre-existing tracking data #973, #975
     - Raise `KeyError` for missing input parameters across helper funcs #966
     - `DLCPosVideo` table now inserts into self after `make` #966
     - Remove unused `PositionVideoSelection` and `PositionVideo` tables #1003
     - Fix SQL query error in `DLCPosV1.fetch_nwb` #1011
+    - Add keyword args to all calls of `convert_to_pixels` #870
+    - Unify `make_video` logic across `DLCPosVideo` and `TrodesVideo` #870
+    - Replace `OutputLogger` context manager with decorator #870
+    - Rename `check_videofile` -> `find_mp4` and `get_video_path` ->
+        `get_video_info` to reflect actual use #870
 - Spikesorting
     - Allow user to set smoothing timescale in `SortedSpikesGroup.get_firing_rate`
         #994
     - Update docstrings #996
     - Remove unused `UnitInclusionParameters` table from `spikesorting.v0` #1003
-    - Fix bug in identification of artifact samples to be zeroed out in `spikesorting.v1.SpikeSorting` #1009
+    - Fix bug in identification of artifact samples to be zeroed out in
+        `spikesorting.v1.SpikeSorting` #1009
 
 ## [0.5.2] (April 22, 2024)
 
@@ -86,11 +102,15 @@
 
 ### Pipelines
 
+- Common:
+    - Add ActivityLog to `common_usage` to track unreferenced utilities. #870
 - Position:
     - Fixes to `environment-dlc.yml` restricting tensortflow #834
     - Video restriction for multicamera epochs #834
     - Fixes to `_convert_mp4` #834
     - Replace deprecated calls to `yaml.safe_load()` #834
+    - Refactoring to reduce redundancy #870
+    - Migrate `OutputLogger` behavior to decorator #870
 - Spikesorting:
     - Increase`spikeinterface` version to >=0.99.1, \<0.100 #852
     - Bug fix in single artifact interval edge case #859

pyproject.toml

@@ -72,6 +72,7 @@ test = [
     "kachery",               # database access
     "kachery-client",
     "kachery-cloud>=0.4.0",
+    "opencv-python-headless", # for headless testing of Qt
     "pre-commit",            # linting
     "pytest",                # unit testing
     "pytest-cov",            # code coverage

src/spyglass/common/common_behav.py

@@ -14,7 +14,7 @@
 from spyglass.common.common_nwbfile import Nwbfile
 from spyglass.common.common_session import Session  # noqa: F401
 from spyglass.common.common_task import TaskEpoch
-from spyglass.settings import video_dir
+from spyglass.settings import test_mode, video_dir
 from spyglass.utils import SpyglassMixin, logger
 from spyglass.utils.nwb_helper_fn import (
     get_all_spatial_series,
@@ -56,8 +56,8 @@ def make(self, keys: Union[List[Dict], dj.Table]):
             keys = [keys]
         if isinstance(keys[0], (dj.Table, dj.expression.QueryExpression)):
             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")
+        nwb_files = set(key.get("nwb_file_name") for key in keys)
+        for nwb_file_name in nwb_files:  # Only unique nwb files
             if not nwb_file_name:
                 raise ValueError("PositionSource.make requires nwb_file_name")
             self.insert_from_nwbfile(nwb_file_name, skip_duplicates=True)
@@ -311,7 +311,7 @@ def make(self, key):
         if associated_files is None:
             logger.info(
                 "Unable to import StateScriptFile: no processing module named "
-                + '"associated_files" found in {nwb_file_name}.'
+                + f'"associated_files" found in {nwb_file_name}.'
             )
             return  # See #849
 
@@ -377,10 +377,12 @@ class VideoFile(SpyglassMixin, dj.Imported):
     def make(self, key):
         self._no_transaction_make(key)
 
-    def _no_transaction_make(self, key, verbose=True):
+    def _no_transaction_make(self, key, verbose=True, skip_duplicates=False):
         if not self.connection.in_transaction:
             self.populate(key)
             return
+        if test_mode:
+            skip_duplicates = True
 
         nwb_file_name = key["nwb_file_name"]
         nwb_file_abspath = Nwbfile.get_abs_path(nwb_file_name)
@@ -404,6 +406,7 @@ def _no_transaction_make(self, key, verbose=True):
                 "interval_list_name": interval_list_name,
             }
         ).fetch1("valid_times")
+
         cam_device_str = r"camera_device (\d+)"
         is_found = False
         for ind, video in enumerate(videos.values()):
@@ -413,28 +416,35 @@ def _no_transaction_make(self, key, verbose=True):
                 # check to see if the times for this video_object are largely
                 # overlapping with the task epoch times
 
-                if len(
+                if not len(
                     interval_list_contains(valid_times, video_obj.timestamps)
                     > 0.9 * len(video_obj.timestamps)
                 ):
-                    nwb_cam_device = video_obj.device.name
-                    # returns whatever was captured in the first group (within the parentheses) of the regular expression -- in this case, 0
-                    key["video_file_num"] = int(
-                        re.match(cam_device_str, nwb_cam_device)[1]
-                    )
-                    camera_name = video_obj.device.camera_name
-                    if CameraDevice & {"camera_name": camera_name}:
-                        key["camera_name"] = video_obj.device.camera_name
-                    else:
-                        raise KeyError(
-                            f"No camera with camera_name: {camera_name} found "
-                            + "in CameraDevice table."
-                        )
-                    key["video_file_object_id"] = video_obj.object_id
-                    self.insert1(
-                        key, skip_duplicates=True, allow_direct_insert=True
+                    continue
+
+                nwb_cam_device = video_obj.device.name
+
+                # returns whatever was captured in the first group (within the
+                # parentheses) of the regular expression - in this case, 0
+
+                key["video_file_num"] = int(
+                    re.match(cam_device_str, nwb_cam_device)[1]
+                )
+                camera_name = video_obj.device.camera_name
+                if CameraDevice & {"camera_name": camera_name}:
+                    key["camera_name"] = video_obj.device.camera_name
+                else:
+                    raise KeyError(
+                        f"No camera with camera_name: {camera_name} found "
+                        + "in CameraDevice table."
                     )
-                    is_found = True
+                key["video_file_object_id"] = video_obj.object_id
+                self.insert1(
+                    key,
+                    skip_duplicates=skip_duplicates,
+                    allow_direct_insert=True,
+                )
+                is_found = True
 
         if not is_found and verbose:
             logger.info(
@@ -443,7 +453,7 @@ def _no_transaction_make(self, key, verbose=True):
             )
 
     @classmethod
-    def update_entries(cls, restrict={}):
+    def update_entries(cls, restrict=True):
         existing_entries = (cls & restrict).fetch("KEY")
         for row in existing_entries:
             if (cls & row).fetch1("camera_name"):
@@ -495,9 +505,11 @@ class PositionIntervalMap(SpyglassMixin, dj.Computed):
     definition = """
     -> IntervalList
     ---
-    position_interval_name: varchar(200)  # name of the corresponding position interval
+    position_interval_name="": varchar(200)  # name of the corresponding interval
     """
 
+    # #849 - Insert null to avoid rerun
+
     def make(self, key):
         self._no_transaction_make(key)
 
@@ -510,18 +522,22 @@ def _no_transaction_make(self, key):
             # if not called in the context of a make function, call its own make function
             self.populate(key)
             return
+        if self & key:
+            return
 
         # *** HARD CODED VALUES ***
         EPSILON = 0.51  # tolerated time diff in bounds across epoch/pos
         no_pop_msg = "CANNOT POPULATE PositionIntervalMap"
 
         nwb_file_name = key["nwb_file_name"]
         pos_intervals = get_pos_interval_list_names(nwb_file_name)
+        null_key = dict(key, position_interval_name="")
+        insert_opts = dict(allow_direct_insert=True, skip_duplicates=True)
 
         # 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}")
+            self.insert1(null_key, **insert_opts)
             return
 
         valid_times = (IntervalList & key).fetch1("valid_times")
@@ -535,7 +551,6 @@ def _no_transaction_make(self, key):
             f"nwb_file_name='{nwb_file_name}' AND interval_list_name=" + "'{}'"
         )
         for pos_interval in pos_intervals:
-            # cbroz: fetch1->fetch. fetch1 would fail w/o result
             pos_times = (IntervalList & restr.format(pos_interval)).fetch(
                 "valid_times"
             )
@@ -558,16 +573,18 @@ 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.warning(
-                f"Found {len(matching_pos_intervals)} pos intervals for {key}; "
-                + f"{no_pop_msg}\n{matching_pos_intervals}"
+                f"{no_pop_msg}. Found {len(matching_pos_intervals)} pos intervals for "
+                + f"\n\t{key}\n\tMatching intervals: {matching_pos_intervals}"
             )
+            self.insert1(null_key, **insert_opts)
             return
 
         # Insert into table
-        key["position_interval_name"] = matching_pos_intervals[0]
-        self.insert1(key, skip_duplicates=True, allow_direct_insert=True)
+        self.insert1(
+            dict(key, position_interval_name=matching_pos_intervals[0]),
+            **insert_opts,
+        )
         logger.info(
             "Populated PosIntervalMap for "
             + f'{nwb_file_name}, {key["interval_list_name"]}'
@@ -609,19 +626,27 @@ def convert_epoch_interval_name_to_position_interval_name(
         )
 
     pos_query = PositionIntervalMap & key
+    pos_str = "position_interval_name"
 
-    if len(pos_query) == 0:
-        if populate_missing:
-            PositionIntervalMap()._no_transaction_make(key)
-            pos_query = PositionIntervalMap & key
+    no_entries = len(pos_query) == 0
+    null_entry = pos_query.fetch(pos_str)[0] == "" if len(pos_query) else False
 
-    if len(pos_query) == 0:
+    if populate_missing and (no_entries or null_entry):
+        if null_entry:
+            pos_query.delete(safemode=False)  # no prompt
+        PositionIntervalMap()._no_transaction_make(key)
+        pos_query = PositionIntervalMap & key
+
+    if pos_query.fetch(pos_str)[0] == "":
         logger.info(f"No position intervals found for {key}")
         return []
 
     if len(pos_query) == 1:
         return pos_query.fetch1("position_interval_name")
 
+    else:
+        raise ValueError(f"Multiple intervals found for {key}: {pos_query}")
+
 
 def get_interval_list_name_from_epoch(nwb_file_name: str, epoch: int) -> str:
     """Returns the interval list name for the given epoch.
@@ -653,13 +678,12 @@ def get_interval_list_name_from_epoch(nwb_file_name: str, epoch: int) -> str:
 
 
 def populate_position_interval_map_session(nwb_file_name: str):
-    for interval_name in (TaskEpoch & {"nwb_file_name": nwb_file_name}).fetch(
-        "interval_list_name"
-    ):
-        with PositionIntervalMap._safe_context():
-            PositionIntervalMap().make(
-                {
-                    "nwb_file_name": nwb_file_name,
-                    "interval_list_name": interval_name,
-                }
-            )
+    # 1. remove redundancy in interval names
+    # 2. let PositionIntervalMap handle transaction context
+    nwb_dict = dict(nwb_file_name=nwb_file_name)
+    intervals = (TaskEpoch & nwb_dict).fetch("interval_list_name")
+    for interval_name in set(intervals):
+        interval_dict = dict(interval_list_name=interval_name)
+        if PositionIntervalMap & interval_dict:
+            continue
+        PositionIntervalMap().make(dict(nwb_dict, **interval_dict))

src/spyglass/common/common_dandi.py

@@ -8,6 +8,8 @@
 import pynwb
 from fsspec.implementations.cached import CachingFileSystem
 
+from spyglass.utils import logger
+
 try:
     import dandi.download
     import dandi.organize

src/spyglass/common/common_device.py

@@ -703,6 +703,8 @@ def prompt_insert(
 
     if table_type:
         table_type += " "
+    else:
+        table_type = ""
 
     logger.info(
         f"{table}{table_type} '{name}' was not found in the"

src/spyglass/common/common_ephys.py

@@ -98,14 +98,10 @@ class Electrode(SpyglassMixin, dj.Imported):
     """
 
     def make(self, key):
-        """Make without transaction
-
-        Allows populate_all_common to work within a single transaction."""
-
         nwb_file_name = key["nwb_file_name"]
         nwb_file_abspath = Nwbfile.get_abs_path(nwb_file_name)
         nwbf = get_nwb_file(nwb_file_abspath)
-        config = get_config(nwb_file_abspath)
+        config = get_config(nwb_file_abspath, calling_table=self.camel_name)
 
         if "Electrode" in config:
             electrode_config_dicts = {
@@ -202,7 +198,7 @@ def create_from_config(cls, nwb_file_name: str):
         """
         nwb_file_abspath = Nwbfile.get_abs_path(nwb_file_name)
         nwbf = get_nwb_file(nwb_file_abspath)
-        config = get_config(nwb_file_abspath)
+        config = get_config(nwb_file_abspath, calling_table=cls.__name__)
         if "Electrode" not in config:
             return  # See #849
 
@@ -323,7 +319,7 @@ def make(self, key):
         # same nwb_object_id
 
         logger.info(
-            f'Importing raw data: Sampling rate:\t{key["sampling_rate"]} Hz\n'
+            f'Importing raw data: Sampling rate:\t{key["sampling_rate"]} Hz\n\t'
             + f'Number of valid intervals:\t{len(interval_dict["valid_times"])}'
         )
 

src/spyglass/common/common_lab.py

@@ -213,8 +213,8 @@ def create_new_team(
             )
             if not query:
                 logger.info(
-                    f"Please add the Google user ID for {team_member} in "
-                    + "LabMember.LabMemberInfo to help manage permissions."
+                    "To help manage permissions in LabMemberInfo, please add Google "
+                    + f"user ID for {team_member}"
                 )
             labteammember_dict = {
                 "team_name": team_name,

src/spyglass/common/common_position.py

@@ -705,7 +705,7 @@ def make_video(
 
                 head_position = head_position_mean[time_ind]
                 head_position = self.convert_to_pixels(
-                    head_position, frame_size, cm_to_pixels
+                    data=head_position, cm_to_pixels=cm_to_pixels
                 )
                 head_orientation = head_orientation_mean[time_ind]