From 025529e041dd22655f1f684a61fd1654e257b7d3 Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@5tk.co>
Date: Fri, 7 Jun 2024 16:30:26 +0300
Subject: [PATCH 01/15] Support the upload of animated GIF files without
 resizing (resolves https://github.com/cortex-lab/alyx/issues/862)

---
 ibllib/pipes/base_tasks.py | 29 +++++++++++++++++++++++++++--
 ibllib/tests/test_pipes.py | 13 +++++++++----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/ibllib/pipes/base_tasks.py b/ibllib/pipes/base_tasks.py
index 5134c4082..22ea727d0 100644
--- a/ibllib/pipes/base_tasks.py
+++ b/ibllib/pipes/base_tasks.py
@@ -5,6 +5,7 @@
 from packaging import version
 from one.webclient import no_cache
 from iblutil.util import flatten
+from skimage.io import ImageCollection
 
 from ibllib.pipes.tasks import Task
 import ibllib.io.session_params as sess_params
@@ -411,6 +412,29 @@ def rename_files(self, symlink_old=False):
             if symlink_old:
                 old_path.symlink_to(new_path)
 
+    @staticmethod
+    def _is_animated_gif(snapshot: Path) -> bool:
+        """
+        Test if image is an animated GIF file.
+
+        Parameters
+        ----------
+        snapshot : pathlib.Path
+            An image filepath to test.
+
+        Returns
+        -------
+        bool
+            True if image is an animated GIF.
+
+        Notes
+        -----
+        This could be achieved more succinctly with `from PIL import Image; Image.open(snapshot).is_animated`,
+        however despite being an indirect dependency, the Pillow library is not in the requirements,
+        whereas skimage is.
+        """
+        return snapshot.suffix == '.gif' and len(ImageCollection(str(snapshot))) > 1
+
     def register_snapshots(self, unlink=False, collection=None):
         """
         Register any photos in the snapshots folder to the session. Typically imaging users will
@@ -438,7 +462,7 @@ def register_snapshots(self, unlink=False, collection=None):
             collection = [p.name for p in self.session_path.glob(collection)]
             # Check whether folders on disk contain '*'; this is to stop an infinite recursion
             assert not any('*' in c for c in collection), 'folders containing asterisks not supported'
-        # If more that one collection exists, register snapshots in each collection
+        # If more than one collection exists, register snapshots in each collection
         if collection and not isinstance(collection, str):
             return flatten(filter(None, [self.register_snapshots(unlink, c) for c in collection]))
         snapshots_path = self.session_path.joinpath(*filter(None, (collection, 'snapshots')))
@@ -452,7 +476,7 @@ def register_snapshots(self, unlink=False, collection=None):
         note = dict(user=self.one.alyx.user, content_type='session', object_id=eid, text='')
 
         notes = []
-        exts = ('.jpg', '.jpeg', '.png', '.tif', '.tiff')
+        exts = ('.jpg', '.jpeg', '.png', '.tif', '.tiff', '.gif')
         for snapshot in filter(lambda x: x.suffix.lower() in exts, snapshots_path.glob('*.*')):
             _logger.debug('Uploading "%s"...', snapshot.relative_to(self.session_path))
             if snapshot.with_suffix('.txt').exists():
@@ -460,6 +484,7 @@ def register_snapshots(self, unlink=False, collection=None):
                     note['text'] = txt_file.read().strip()
             else:
                 note['text'] = ''
+            note['width'] = 'orig' if self._is_animated_gif(snapshot) else None
             with open(snapshot, 'rb') as img_file:
                 files = {'image': img_file}
                 notes.append(self.one.alyx.rest('notes', 'create', data=note, files=files))
diff --git a/ibllib/tests/test_pipes.py b/ibllib/tests/test_pipes.py
index f46577d9b..d2b321ee7 100644
--- a/ibllib/tests/test_pipes.py
+++ b/ibllib/tests/test_pipes.py
@@ -702,7 +702,7 @@ def test_rename_files(self):
         (folder := self.session_path.joinpath('snapshots')).mkdir()
         folder.joinpath('snap.PNG').touch()
         collection = 'raw_task_data'
-        for i, ext in enumerate(['tif', 'jpg']):
+        for i, ext in enumerate(['tif', 'jpg', 'gif']):
             (p := self.session_path.joinpath(f'{collection}_{i:02}', 'snapshots')).mkdir(parents=True)
             p.joinpath(f'snapshot.{ext}').touch()
         # Stuff with text note
@@ -713,15 +713,20 @@ def test_rename_files(self):
             fp.write('bar')
 
         task = RegisterRawDataTask(self.session_path, one=self.one)
+        # Mock the _is_animated_gif function to return true for any GIF file
         with mock.patch.object(self.one.alyx, 'rest') as rest, \
-                mock.patch.object(self.one, 'path2eid', return_value=str(uuid4())):
+                mock.patch.object(self.one, 'path2eid', return_value=str(uuid4())), \
+                mock.patch.object(task, '_is_animated_gif', side_effect=lambda x: x.suffix == '.gif'):
             task.register_snapshots(collection=['', f'{collection}*'])
-            self.assertEqual(4, rest.call_count)
+            self.assertEqual(5, rest.call_count)
             files = []
             for args, kwargs in rest.call_args_list:
                 self.assertEqual(('notes', 'create'), args)
                 files.append(Path(kwargs['files']['image'].name).name)
-            expected = ('snap.PNG', 'pic.jpeg', 'snapshot.tif', 'snapshot.jpg')
+                width = kwargs['data'].get('width')
+                # Test that original size passed as width only for gif file
+                self.assertEqual('orig', width) if files[-1].endswith('gif') else self.assertIsNone(width)
+            expected = ('snap.PNG', 'pic.jpeg', 'snapshot.tif', 'snapshot.jpg', 'snapshot.gif')
             self.assertCountEqual(expected, files)
 
 

From 37f54e0b964d8c8d33ce734b2a138e424cfa6aaa Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@3tk.co>
Date: Thu, 13 Jun 2024 11:57:21 +0100
Subject: [PATCH 02/15] Issue #769 - Divert all sessions through dynamic
 pipeline

---
 ibllib/pipes/local_server.py | 24 ++++++-----------------
 ibllib/tests/test_pipes.py   | 37 ++++++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/ibllib/pipes/local_server.py b/ibllib/pipes/local_server.py
index 42edc3b34..6574f88f1 100644
--- a/ibllib/pipes/local_server.py
+++ b/ibllib/pipes/local_server.py
@@ -19,10 +19,10 @@
 from one.remote.globus import get_lab_from_endpoint_id, get_local_endpoint_id
 
 from ibllib import __version__ as ibllib_version
-from ibllib.io.extractors.base import get_pipeline, get_task_protocol, get_session_extractor_type
+from ibllib.io.extractors.base import get_pipeline, get_session_extractor_type
 from ibllib.pipes import tasks, training_preprocessing, ephys_preprocessing
 from ibllib.time import date2isostr
-from ibllib.oneibl.registration import IBLRegistrationClient, register_session_raw_data, get_lab
+from ibllib.oneibl.registration import IBLRegistrationClient
 from ibllib.oneibl.data_handlers import get_local_data_repository
 from ibllib.io.session_params import read_params
 from ibllib.pipes.dynamic_pipeline import make_pipeline, acquisition_description_legacy_session
@@ -88,7 +88,7 @@ def report_health(one):
         one.alyx.json_field_update(endpoint='data-repository', uuid=dr['name'], field_name='json', data=status)
 
 
-def job_creator(root_path, one=None, dry=False, rerun=False, max_md5_size=None):
+def job_creator(root_path, one=None, dry=False, rerun=False):
     """
     Create new sessions and pipelines.
 
@@ -108,8 +108,6 @@ def job_creator(root_path, one=None, dry=False, rerun=False, max_md5_size=None):
         If true, simply log the session_path(s) found, without registering anything.
     rerun : bool
         If true and session pipeline tasks already exist, set them all to waiting.
-    max_md5_size : int
-        (legacy sessions) The maximum file size to calculate the MD5 hash sum for.
 
     Returns
     -------
@@ -134,21 +132,11 @@ def job_creator(root_path, one=None, dry=False, rerun=False, max_md5_size=None):
             # if the subject doesn't exist in the database, skip
             rc.register_session(session_path, file_list=False)
 
-            # See if we need to create a dynamic pipeline
-            experiment_description_file = read_params(session_path)
-            if experiment_description_file is not None:
-                pipe = make_pipeline(session_path, one=one)
-            else:
+            # NB: all sessions now extracted using dynamic pipeline
+            if read_params(session_path) is None:
                 # Create legacy experiment description file
                 acquisition_description_legacy_session(session_path, save=True)
-                lab = get_lab(session_path, one.alyx)  # Can be set to None to do this Alyx-side if using ONE v1.20.1
-                _, dsets = register_session_raw_data(session_path, one=one, max_md5_size=max_md5_size, labs=lab)
-                if dsets:
-                    all_datasets.extend(dsets)
-                pipe = _get_pipeline_class(session_path, one)
-                if pipe is None:
-                    task_protocol = get_task_protocol(session_path)
-                    _logger.info(f'Session task protocol {task_protocol} has no matching pipeline pattern {session_path}')
+            pipe = make_pipeline(session_path, one=one)
             if rerun:
                 rerun__status__in = '__all__'
             else:
diff --git a/ibllib/tests/test_pipes.py b/ibllib/tests/test_pipes.py
index f46577d9b..45df145ee 100644
--- a/ibllib/tests/test_pipes.py
+++ b/ibllib/tests/test_pipes.py
@@ -12,7 +12,7 @@
 import string
 from uuid import uuid4
 
-from one.api import ONE
+from one.api import ONE, OneAlyx
 import iblutil.io.params as iopar
 from packaging.version import Version, InvalidVersion
 
@@ -20,18 +20,51 @@
 import ibllib.tests.fixtures.utils as fu
 from ibllib.pipes import misc
 from ibllib.pipes.misc import sleepless
+from ibllib.pipes import local_server
 from ibllib.tests import TEST_DB
 import ibllib.pipes.scan_fix_passive_files as fix
 from ibllib.pipes.base_tasks import RegisterRawDataTask
 from ibllib.pipes.ephys_preprocessing import SpikeSorting
 
 
+class TestLocalServer(unittest.TestCase):
+    """Tests for the ibllib.pipes.local_server module."""
+    def setUp(self):
+        tmp = tempfile.TemporaryDirectory()
+        self.tmpdir = Path(tmp.name)
+        self.addCleanup(tmp.cleanup)
+        raw_behaviour_data = fu.create_fake_raw_behavior_data_folder(self.tmpdir / 'subject/2020-01-01/001', task='ephys')
+        raw_behaviour_data.parent.joinpath('raw_session.flag').touch()
+        fu.populate_task_settings(raw_behaviour_data, patch={'PYBPOD_PROTOCOL': '_iblrig_ephysChoiceWorld5.2.1'})
+        raw_behaviour_data = fu.create_fake_raw_behavior_data_folder(self.tmpdir / 'subject/2020-01-01/002')
+        raw_behaviour_data.parent.joinpath('raw_session.flag').touch()
+        fu.populate_task_settings(raw_behaviour_data, patch={'PYBPOD_PROTOCOL': 'ephys_optoChoiceWorld6.0.1'})
+
+    @mock.patch('ibllib.pipes.local_server.IBLRegistrationClient')
+    @mock.patch('ibllib.pipes.local_server.make_pipeline')
+    def test_job_creator(self, pipeline_mock, _):
+        """Test the job_creator function.
+
+        This test was created after retiring the legacy pipeline. Here we test that an experiment
+        description file is created for each legacy session, followed by dynamic pipeline creation.
+        The second session tests the behaviour of a legacy pipeline with no corresponding experiment
+        description template.  For these sessions we will simply update acquisition_description_legacy_session
+        to add support.
+        """
+        one = mock.Mock(spec=OneAlyx)
+        with self.assertLogs(local_server.__name__, 'ERROR') as log:
+            pipes, _ = local_server.job_creator(self.tmpdir, one=one)
+        self.assertIn("KeyError: 'biased_opto'", log.records[0].getMessage())
+        self.assertEqual(len(pipes), 1)
+        pipeline_mock.assert_called_once()
+
+
 class TestExtractors2Tasks(unittest.TestCase):
 
     def test_task_to_pipeline(self):
         dd = ibllib.io.extractors.base._get_task_types_json_config()
         types = list(set([dd[k] for k in dd]))
-        # makes sure that for every defined task type there is an acutal pipeline
+        # makes sure that for every defined task type there is an actual pipeline
         for type in types:
             assert ibllib.io.extractors.base._get_pipeline_from_task_type(type)
             print(type, ibllib.io.extractors.base._get_pipeline_from_task_type(type))

From e9cecf86958e0996f43a97a7dc49d592a234c9ab Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@3tk.co>
Date: Thu, 13 Jun 2024 12:05:56 +0100
Subject: [PATCH 03/15] Schedule removal of legacy pipeline code

---
 ibllib/tests/test_pipes.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ibllib/tests/test_pipes.py b/ibllib/tests/test_pipes.py
index 45df145ee..719b2c64e 100644
--- a/ibllib/tests/test_pipes.py
+++ b/ibllib/tests/test_pipes.py
@@ -11,6 +11,7 @@
 import random
 import string
 from uuid import uuid4
+from datetime import datetime
 
 from one.api import ONE, OneAlyx
 import iblutil.io.params as iopar
@@ -58,6 +59,16 @@ def test_job_creator(self, pipeline_mock, _):
         self.assertEqual(len(pipes), 1)
         pipeline_mock.assert_called_once()
 
+        # In September 2024, the legacy pipeline will be removed. This entails removing the
+        # code in pipes.training_preprocessing and pipes.ephys_preprocessing, as well as the
+        # code in qc.task_extractors and the extract_all functions in the io.extractors modules.
+        # NB: some tasks such as ephys opto do not have experiment description templates and some
+        # legacy tasks are imported for use in the dynamic pipeline.
+        self.assertFalse(
+            datetime.today() > datetime(2024, 9, 1),
+            'Legacy pipeline code scheduled to be removed after 2024-09-01'
+        )
+
 
 class TestExtractors2Tasks(unittest.TestCase):
 

From e55b9ce8e59668dc83cb503634b361a84f9e1537 Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@3tk.co>
Date: Thu, 13 Jun 2024 13:50:11 +0100
Subject: [PATCH 04/15] Convert snapshot tif files to png before upload

---
 ibllib/pipes/base_tasks.py      | 33 ++++++++++++++++++++++++++++++++-
 ibllib/tests/test_base_tasks.py |  7 ++++---
 ibllib/tests/test_pipes.py      |  4 +++-
 3 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/ibllib/pipes/base_tasks.py b/ibllib/pipes/base_tasks.py
index 22ea727d0..364abf1c7 100644
--- a/ibllib/pipes/base_tasks.py
+++ b/ibllib/pipes/base_tasks.py
@@ -5,7 +5,8 @@
 from packaging import version
 from one.webclient import no_cache
 from iblutil.util import flatten
-from skimage.io import ImageCollection
+import matplotlib.image
+from skimage.io import ImageCollection, imread
 
 from ibllib.pipes.tasks import Task
 import ibllib.io.session_params as sess_params
@@ -435,6 +436,25 @@ def _is_animated_gif(snapshot: Path) -> bool:
         """
         return snapshot.suffix == '.gif' and len(ImageCollection(str(snapshot))) > 1
 
+    @staticmethod
+    def _save_as_png(snapshot: Path) -> Path:
+        """
+        Save an image to PNG format.
+
+        Parameters
+        ----------
+        snapshot : pathlib.Path
+            An image filepath to convert.
+
+        Returns
+        -------
+        pathlib.Path
+            The new PNG image filepath.
+        """
+        img = imread(snapshot, as_gray=True)
+        matplotlib.image.imsave(snapshot.with_suffix('.png'), img, cmap='gray')
+        return snapshot.with_suffix('.png')
+
     def register_snapshots(self, unlink=False, collection=None):
         """
         Register any photos in the snapshots folder to the session. Typically imaging users will
@@ -455,6 +475,12 @@ def register_snapshots(self, unlink=False, collection=None):
         -------
         list of dict
             The newly registered Alyx notes.
+
+        Notes
+        -----
+        - Animated GIF files are not resized and therefore may take up significant space on the database.
+        - TIFF files are converted to PNG format before upload. The original file is not replaced.
+        - JPEG and PNG files are resized by Alyx.
         """
         collection = getattr(self, 'device_collection', None) if collection is None else collection
         collection = collection or ''  # If not defined, use no collection
@@ -478,6 +504,11 @@ def register_snapshots(self, unlink=False, collection=None):
         notes = []
         exts = ('.jpg', '.jpeg', '.png', '.tif', '.tiff', '.gif')
         for snapshot in filter(lambda x: x.suffix.lower() in exts, snapshots_path.glob('*.*')):
+            if snapshot.suffix in ('.tif', '.tiff') and not snapshot.with_suffix('.png').exists():
+                _logger.debug('converting "%s" to png...', snapshot.relative_to(self.session_path))
+                snapshot = self._save_as_png(snapshot_tif := snapshot)
+                if unlink:
+                    snapshot_tif.unlink()
             _logger.debug('Uploading "%s"...', snapshot.relative_to(self.session_path))
             if snapshot.with_suffix('.txt').exists():
                 with open(snapshot.with_suffix('.txt'), 'r') as txt_file:
diff --git a/ibllib/tests/test_base_tasks.py b/ibllib/tests/test_base_tasks.py
index a6dab7edf..f53a58612 100644
--- a/ibllib/tests/test_base_tasks.py
+++ b/ibllib/tests/test_base_tasks.py
@@ -36,9 +36,9 @@ def setUpClass(cls) -> None:
 
         # Add a couple of images
         cls.session_path.joinpath('snapshots').mkdir(parents=True)
-        for ext in ('.PNG', '.tif'):
+        for i, ext in enumerate(('.PNG', '.tif')):
             plt.imshow(np.random.random((7, 7)))
-            plt.savefig(cls.session_path.joinpath('snapshots', 'foo').with_suffix(ext))
+            plt.savefig(cls.session_path.joinpath('snapshots', f'foo_{i}').with_suffix(ext))
             plt.close()
 
     def test_register_snapshots(self):
@@ -46,7 +46,7 @@ def test_register_snapshots(self):
 
         A more thorough test for this exists in ibllib.tests.test_pipes.TestRegisterRawDataTask.
         This test does not mock REST (and therefore requires a test database), while the other does.
-        This test could be removed as it's rather redundant.
+        This test also works on actual image data, testing the conversion from tif to png.
         """
         task = base_tasks.RegisterRawDataTask(self.session_path, one=self.one)
         notes = task.register_snapshots()
@@ -54,6 +54,7 @@ def test_register_snapshots(self):
         self.assertTrue(self.session_path.joinpath('snapshots').exists())
         task.register_snapshots(unlink=True)
         self.assertFalse(self.session_path.joinpath('snapshots').exists())
+        self.assertTrue(all(n['image'].lower().endswith('.png') for n in notes), 'failed to convert tif to png')
 
     def test_rename_files(self):
         collection = 'raw_sync_data'
diff --git a/ibllib/tests/test_pipes.py b/ibllib/tests/test_pipes.py
index d2b321ee7..d470551c9 100644
--- a/ibllib/tests/test_pipes.py
+++ b/ibllib/tests/test_pipes.py
@@ -697,6 +697,7 @@ def test_rename_files(self):
         """Test upload of snapshots.
 
         Another test for this exists in ibllib.tests.test_base_tasks.TestRegisterRawDataTask.
+        This test does not work on real files and works without a test db.
         """
         # Add base dir snapshot
         (folder := self.session_path.joinpath('snapshots')).mkdir()
@@ -716,6 +717,7 @@ def test_rename_files(self):
         # Mock the _is_animated_gif function to return true for any GIF file
         with mock.patch.object(self.one.alyx, 'rest') as rest, \
                 mock.patch.object(self.one, 'path2eid', return_value=str(uuid4())), \
+                mock.patch.object(task, '_save_as_png', side_effect=lambda x: x.with_suffix('.png').touch()), \
                 mock.patch.object(task, '_is_animated_gif', side_effect=lambda x: x.suffix == '.gif'):
             task.register_snapshots(collection=['', f'{collection}*'])
             self.assertEqual(5, rest.call_count)
@@ -726,7 +728,7 @@ def test_rename_files(self):
                 width = kwargs['data'].get('width')
                 # Test that original size passed as width only for gif file
                 self.assertEqual('orig', width) if files[-1].endswith('gif') else self.assertIsNone(width)
-            expected = ('snap.PNG', 'pic.jpeg', 'snapshot.tif', 'snapshot.jpg', 'snapshot.gif')
+            expected = ('snap.PNG', 'pic.jpeg', 'snapshot.png', 'snapshot.jpg', 'snapshot.gif')
             self.assertCountEqual(expected, files)
 
 

From eb771cf20ae3c66fb7493c748790e50dbf2c1d6e Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@3tk.co>
Date: Thu, 13 Jun 2024 18:16:02 +0100
Subject: [PATCH 05/15] Format test

---
 ibllib/tests/test_pipes.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ibllib/tests/test_pipes.py b/ibllib/tests/test_pipes.py
index d470551c9..f0eb6fe4e 100644
--- a/ibllib/tests/test_pipes.py
+++ b/ibllib/tests/test_pipes.py
@@ -715,9 +715,10 @@ def test_rename_files(self):
 
         task = RegisterRawDataTask(self.session_path, one=self.one)
         # Mock the _is_animated_gif function to return true for any GIF file
+        as_png_side_effect = lambda x: x.with_suffix('.png').touch() or x.with_suffix('.png')  # noqa
         with mock.patch.object(self.one.alyx, 'rest') as rest, \
                 mock.patch.object(self.one, 'path2eid', return_value=str(uuid4())), \
-                mock.patch.object(task, '_save_as_png', side_effect=lambda x: x.with_suffix('.png').touch()), \
+                mock.patch.object(task, '_save_as_png', side_effect=as_png_side_effect), \
                 mock.patch.object(task, '_is_animated_gif', side_effect=lambda x: x.suffix == '.gif'):
             task.register_snapshots(collection=['', f'{collection}*'])
             self.assertEqual(5, rest.call_count)

From e0b485d61aa530dadd684faa8864b4fa0ece0003 Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@3tk.co>
Date: Fri, 14 Jun 2024 11:33:35 +0100
Subject: [PATCH 06/15] sync_label function returns software dependent sync
 name

---
 ibllib/pipes/dynamic_pipeline.py | 77 +++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 30 deletions(-)

diff --git a/ibllib/pipes/dynamic_pipeline.py b/ibllib/pipes/dynamic_pipeline.py
index 7024d4ce3..fdb81cdbd 100644
--- a/ibllib/pipes/dynamic_pipeline.py
+++ b/ibllib/pipes/dynamic_pipeline.py
@@ -139,6 +139,34 @@ def get_acquisition_description(protocol):
     return acquisition_description
 
 
+def _sync_label(sync, acquisition_software=None, **_):
+    """
+    Returns the sync label based on the sync type and acquisition software.
+
+    The 'sync' usually refers to the DAQ type, e.g. 'nidq', 'tdms', 'bpod'.
+    The 'acquisition_software' refers to the software used to acquire the data, e.g.
+    for an NI DAQ, options include 'spikeglx' and 'timeline'.  Both of these affect
+    how the data are loaded and extracted, and therefore which tasks to use.
+
+    The naming convention here is not ideal, and may be changed in the future.
+
+    Parameters
+    ----------
+    sync : str
+        The sync type, e.g. 'nidq', 'tdms', 'bpod'.
+    acquisition_software : str
+        The acquisition software used to acquire the sync data.
+
+    Returns
+    -------
+    str
+        The sync label for determining the extractor tasks.
+    """
+    if sync == 'nidq' and acquisition_software == 'timeline':
+        return 'timeline'
+    return sync
+
+
 def make_pipeline(session_path, **pkwargs):
     """
     Creates a pipeline of extractor tasks from a session's experiment description file.
@@ -172,27 +200,28 @@ def make_pipeline(session_path, **pkwargs):
 
     # Syncing tasks
     (sync, sync_args), = acquisition_description['sync'].items()
+    sync_label = _sync_label(sync, **sync_args)  # get the format of the DAQ data. This informs the extractor task
     sync_args['sync_collection'] = sync_args.pop('collection')  # rename the key so it matches task run arguments
     sync_args['sync_ext'] = sync_args.pop('extension', None)
     sync_args['sync_namespace'] = sync_args.pop('acquisition_software', None)
     sync_kwargs = {'sync': sync, **sync_args}
     sync_tasks = []
-    if sync == 'nidq' and sync_args['sync_collection'] == 'raw_ephys_data':
+    if sync_label == 'nidq' and sync_args['sync_collection'] == 'raw_ephys_data':
         tasks['SyncRegisterRaw'] = type('SyncRegisterRaw', (etasks.EphysSyncRegisterRaw,), {})(**kwargs, **sync_kwargs)
         tasks[f'SyncPulses_{sync}'] = type(f'SyncPulses_{sync}', (etasks.EphysSyncPulses,), {})(
             **kwargs, **sync_kwargs, parents=[tasks['SyncRegisterRaw']])
         sync_tasks = [tasks[f'SyncPulses_{sync}']]
-    elif sync_args['sync_namespace'] == 'timeline':
+    elif sync_label == 'timeline':
         tasks['SyncRegisterRaw'] = type('SyncRegisterRaw', (stasks.SyncRegisterRaw,), {})(**kwargs, **sync_kwargs)
-    elif sync == 'nidq':
+    elif sync_label == 'nidq':
         tasks['SyncRegisterRaw'] = type('SyncRegisterRaw', (stasks.SyncMtscomp,), {})(**kwargs, **sync_kwargs)
         tasks[f'SyncPulses_{sync}'] = type(f'SyncPulses_{sync}', (stasks.SyncPulses,), {})(
             **kwargs, **sync_kwargs, parents=[tasks['SyncRegisterRaw']])
         sync_tasks = [tasks[f'SyncPulses_{sync}']]
-    elif sync == 'tdms':
+    elif sync_label == 'tdms':
         tasks['SyncRegisterRaw'] = type('SyncRegisterRaw', (stasks.SyncRegisterRaw,), {})(**kwargs, **sync_kwargs)
-    elif sync == 'bpod':
-        pass  # ATM we don't have anything for this not sure it will be needed in the future
+    elif sync_label == 'bpod':
+        pass  # ATM we don't have anything for this; it may not be needed in the future
 
     # Behavior tasks
     task_protocols = acquisition_description.get('tasks', [])
@@ -212,16 +241,16 @@ def make_pipeline(session_path, **pkwargs):
                 # Assume previous task in the list is parent
                 parents = [] if j == 0 else [tasks[task_name]]
                 # Make sure extractor and sync task don't collide
-                for sync_option in ('nidq', 'bpod'):
-                    if sync_option in extractor.lower() and not sync == sync_option:
-                        raise ValueError(f'Extractor "{extractor}" and sync "{sync}" do not match')
+                for sync_option in ('nidq', 'bpod', 'timeline'):
+                    if sync_option in extractor.lower() and not sync_label == sync_option:
+                        raise ValueError(f'Extractor "{extractor}" and sync "{sync_label}" do not match')
                 # TODO Assert sync_label correct here (currently unused)
                 # Look for the extractor in the behavior extractors module
                 if hasattr(btasks, extractor):
                     task = getattr(btasks, extractor)
                 # This may happen that the extractor is tied to a specific sync task: look for TrialsChoiceWorldBpod for example
-                elif hasattr(btasks, extractor + sync.capitalize()):
-                    task = getattr(btasks, extractor + sync.capitalize())
+                elif hasattr(btasks, extractor + sync_label.capitalize()):
+                    task = getattr(btasks, extractor + sync_label.capitalize())
                 else:
                     # lookup in the project extraction repo if we find an extractor class
                     import projects.extraction_tasks
@@ -252,26 +281,14 @@ def make_pipeline(session_path, **pkwargs):
                 registration_class = btasks.PassiveRegisterRaw
                 behaviour_class = btasks.PassiveTask
                 compute_status = False
-            elif sync_kwargs['sync'] == 'bpod':
-                if 'habituation' in protocol:
-                    registration_class = btasks.HabituationRegisterRaw
-                    behaviour_class = btasks.HabituationTrialsBpod
-                    compute_status = False
-                else:
-                    registration_class = btasks.TrialRegisterRaw
-                    behaviour_class = btasks.ChoiceWorldTrialsBpod
-                    compute_status = True
-            elif sync_kwargs['sync'] == 'nidq':
-                if 'habituation' in protocol:
-                    registration_class = btasks.HabituationRegisterRaw
-                    behaviour_class = btasks.HabituationTrialsNidq
-                    compute_status = False
-                else:
-                    registration_class = btasks.TrialRegisterRaw
-                    behaviour_class = btasks.ChoiceWorldTrialsNidq
-                    compute_status = True
+            elif 'habituation' in protocol:
+                registration_class = btasks.HabituationRegisterRaw
+                behaviour_class = getattr(btasks, 'HabituationTrials' + sync_label.capitalize())
+                compute_status = False
             else:
-                raise NotImplementedError
+                registration_class = btasks.TrialRegisterRaw
+                behaviour_class = getattr(btasks, 'ChoiceWorldTrials' + sync_label.capitalize())
+                compute_status = True
             tasks[f'RegisterRaw_{protocol}_{i:02}'] = type(f'RegisterRaw_{protocol}_{i:02}', (registration_class,), {})(
                 **kwargs, **task_kwargs)
             parents = [tasks[f'RegisterRaw_{protocol}_{i:02}']] + sync_tasks

From db873352f9e7b99ef74b1b7dc95212d752ea933e Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@3tk.co>
Date: Mon, 17 Jun 2024 13:10:10 +0300
Subject: [PATCH 07/15] Test _sync_label function

---
 ibllib/tests/test_dynamic_pipeline.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ibllib/tests/test_dynamic_pipeline.py b/ibllib/tests/test_dynamic_pipeline.py
index 41420c674..53e136f30 100644
--- a/ibllib/tests/test_dynamic_pipeline.py
+++ b/ibllib/tests/test_dynamic_pipeline.py
@@ -134,3 +134,13 @@ def test_get_trials_tasks(self):
             # Should handle absent trials tasks
             pipeline.tasks.pop('FooBarTrials')
             self.assertEqual([], dyn.get_trials_tasks(self.session_path_legacy))
+
+
+class TestMisc(unittest.TestCase):
+    """Test miscellaneous functions in pipes.dynamic_pipeline."""
+    def test_sync_label(self):
+        """Test pipes.dynamic_pipeline._sync_label function."""
+        self.assertEqual('nidq', dyn._sync_label('nidq'))
+        self.assertEqual('timeline', dyn._sync_label('nidq', acquisition_software='timeline'))
+        self.assertEqual('nidq', dyn._sync_label('nidq', acquisition_software='spikeglx'))
+        self.assertEqual('tdms', dyn._sync_label('tdms'))

From af86ce6f83cc5cce948dc37a4a8f93eeb5d477a4 Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Mon, 17 Jun 2024 16:38:27 +0100
Subject: [PATCH 08/15] Optional task replay files in PassiveRegisterRaw;
 rename PassiveTask -> PassiveTaskNidq

---
 ibllib/pipes/behavior_tasks.py         | 12 ++++++------
 ibllib/pipes/dynamic_pipeline.py       |  2 +-
 ibllib/tests/qc/test_task_qc_viewer.py |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/ibllib/pipes/behavior_tasks.py b/ibllib/pipes/behavior_tasks.py
index b230e16b1..b4105affa 100644
--- a/ibllib/pipes/behavior_tasks.py
+++ b/ibllib/pipes/behavior_tasks.py
@@ -184,17 +184,17 @@ def signature(self):
         signature = {
             'input_files': [],
             'output_files': [('_iblrig_taskSettings.raw.*', self.collection, True),
-                             ('_iblrig_encoderEvents.raw*', self.collection, True),
-                             ('_iblrig_encoderPositions.raw*', self.collection, True),
-                             ('_iblrig_encoderTrialInfo.raw*', self.collection, True),
-                             ('_iblrig_stimPositionScreen.raw*', self.collection, True),
-                             ('_iblrig_syncSquareUpdate.raw*', self.collection, True),
+                             ('_iblrig_encoderEvents.raw*', self.collection, False),
+                             ('_iblrig_encoderPositions.raw*', self.collection, False),
+                             ('_iblrig_encoderTrialInfo.raw*', self.collection, False),
+                             ('_iblrig_stimPositionScreen.raw*', self.collection, False),
+                             ('_iblrig_syncSquareUpdate.raw*', self.collection, False),
                              ('_iblrig_RFMapStim.raw*', self.collection, True)]
         }
         return signature
 
 
-class PassiveTask(base_tasks.BehaviourTask):
+class PassiveTaskNidq(base_tasks.BehaviourTask):
     priority = 90
     job_size = 'small'
 
diff --git a/ibllib/pipes/dynamic_pipeline.py b/ibllib/pipes/dynamic_pipeline.py
index fdb81cdbd..1d15646b2 100644
--- a/ibllib/pipes/dynamic_pipeline.py
+++ b/ibllib/pipes/dynamic_pipeline.py
@@ -279,7 +279,7 @@ def make_pipeline(session_path, **pkwargs):
             # -   choice_world_habituation
             if 'passiveChoiceWorld' in protocol:
                 registration_class = btasks.PassiveRegisterRaw
-                behaviour_class = btasks.PassiveTask
+                behaviour_class = getattr(btasks, 'PassiveTask' + sync_label.capitalize())
                 compute_status = False
             elif 'habituation' in protocol:
                 registration_class = btasks.HabituationRegisterRaw
diff --git a/ibllib/tests/qc/test_task_qc_viewer.py b/ibllib/tests/qc/test_task_qc_viewer.py
index a3b1d61b4..9873e105f 100644
--- a/ibllib/tests/qc/test_task_qc_viewer.py
+++ b/ibllib/tests/qc/test_task_qc_viewer.py
@@ -8,7 +8,7 @@
 
 from ibllib.pipes.ephys_preprocessing import EphysTrials
 from ibllib.pipes.training_preprocessing import TrainingTrials
-from ibllib.pipes.behavior_tasks import HabituationTrialsBpod, ChoiceWorldTrialsNidq, ChoiceWorldTrialsBpod, PassiveTask
+from ibllib.pipes.behavior_tasks import HabituationTrialsBpod, ChoiceWorldTrialsNidq, ChoiceWorldTrialsBpod, PassiveTaskNidq
 from ibllib.qc.task_qc_viewer.task_qc import get_bpod_trials_task, show_session_task_qc, QcFrame
 from ibllib.qc.task_metrics import TaskQC
 from ibllib.tests import TEST_DB
@@ -65,7 +65,7 @@ def test_show_session_task_qc(self, trials_tasks_mock, run_app_mock):
         self.assertRaises(TypeError, show_session_task_qc, session_path, one=self.one, protocol_number=-2)
         self.assertRaises(ValueError, show_session_task_qc, session_path, one=self.one, protocol_number=1)
 
-        passive_task = PassiveTask('foo/bar', protocol='_iblrig_passiveChoiceWorld', protocol_number=0)
+        passive_task = PassiveTaskNidq('foo/bar', protocol='_iblrig_passiveChoiceWorld', protocol_number=0)
         trials_tasks_mock.return_value = [passive_task]
         self.assertRaises(ValueError, show_session_task_qc, session_path, one=self.one, protocol_number=0)
         self.assertRaises(ValueError, show_session_task_qc, session_path, one=self.one)

From e14acc2c4882fc28e04d1e64abf0bfee47a0b0db Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Mon, 17 Jun 2024 17:14:55 +0100
Subject: [PATCH 09/15] see if task replay extraction should be skipped

---
 ibllib/io/extractors/ephys_passive.py | 43 ++++++++++++++++++++-------
 ibllib/pipes/behavior_tasks.py        |  8 ++---
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/ibllib/io/extractors/ephys_passive.py b/ibllib/io/extractors/ephys_passive.py
index 5bd51b909..4fbbfc868 100644
--- a/ibllib/io/extractors/ephys_passive.py
+++ b/ibllib/io/extractors/ephys_passive.py
@@ -86,7 +86,7 @@ def _load_passive_session_fixtures(session_path: str, task_collection: str = 'ra
     return fixture
 
 
-def _load_task_protocol(session_path: str, task_collection: str = 'raw_passive_data') -> str:
+def _load_task_version(session_path: str, task_collection: str = 'raw_passive_data') -> str:
     """Find the IBL rig version used for the session
 
     :param session_path: the path to a session
@@ -94,7 +94,6 @@ def _load_task_protocol(session_path: str, task_collection: str = 'raw_passive_d
     :return: ibl rig task protocol version
     :rtype: str
 
-    FIXME This function has a misleading name
     """
     settings = rawio.load_settings(session_path, task_collection=task_collection)
     ses_ver = settings["IBLRIG_VERSION"]
@@ -102,6 +101,24 @@ def _load_task_protocol(session_path: str, task_collection: str = 'raw_passive_d
     return ses_ver
 
 
+def skip_task_replay(session_path: str, task_collection: str = 'raw_passive_data') -> bool:
+    """Find whether the task replay portion of the passive stimulus has been shown
+
+    :param session_path: the path to a session
+    :type session_path: str
+    :param task_collection: collection containing task data
+    :type task_collection: str
+    :return: whether or not the task replay has been run
+    :rtype: bool
+    """
+
+    settings = rawio.load_settings(session_path, task_collection=task_collection)
+    # Attempt to see if SKIP_EVENT_REPLAY is available, if not assume we do have task replay
+    skip_replay = settings.get('SKIP_EVENT_REPLAY', False)
+
+    return skip_replay
+
+
 def _load_passive_stim_meta() -> dict:
     """load_passive_stim_meta Loads the passive protocol metadata
 
@@ -536,7 +553,7 @@ def extract_task_replay(
     bpod = ephys_fpga.get_sync_fronts(sync, sync_map["bpod"], tmin=treplay[0], tmax=treplay[1])
     passiveValve_intervals = _extract_passiveValve_intervals(bpod)
 
-    task_version = _load_task_protocol(session_path, task_collection)
+    task_version = _load_task_version(session_path, task_collection)
     audio = ephys_fpga.get_sync_fronts(sync, sync_map["audio"], tmin=treplay[0], tmax=treplay[1])
     passiveTone_intervals, passiveNoise_intervals = _extract_passiveAudio_intervals(audio, task_version)
 
@@ -588,7 +605,7 @@ def extract_replay_debug(
     passiveValve_intervals = _extract_passiveValve_intervals(bpod)
     plot_valve_times(passiveValve_intervals, ax=ax)
 
-    task_version = _load_task_protocol(session_path, task_collection)
+    task_version = _load_task_version(session_path, task_collection)
     audio = ephys_fpga.get_sync_fronts(sync, sync_map["audio"], tmin=treplay[0])
     passiveTone_intervals, passiveNoise_intervals = _extract_passiveAudio_intervals(audio, task_version)
     plot_audio_times(passiveTone_intervals, passiveNoise_intervals, ax=ax)
@@ -647,13 +664,19 @@ def _extract(self, sync_collection: str = 'raw_ephys_data', task_collection: str
             log.error(f"Failed to extract RFMapping datasets: {e}")
             passiveRFM_times = None
 
-        try:
-            (passiveGabor_df, passiveStims_df,) = extract_task_replay(
-                self.session_path, sync_collection=sync_collection, task_collection=task_collection, sync=sync,
-                sync_map=sync_map, treplay=treplay)
-        except Exception as e:
-            log.error(f"Failed to extract task replay stimuli: {e}")
+        skip_replay = skip_task_replay(self.session_path, task_collection)
+        if not skip_replay:
+            try:
+                (passiveGabor_df, passiveStims_df,) = extract_task_replay(
+                    self.session_path, sync_collection=sync_collection, task_collection=task_collection, sync=sync,
+                    sync_map=sync_map, treplay=treplay)
+            except Exception as e:
+                log.error(f"Failed to extract task replay stimuli: {e}")
+                passiveGabor_df, passiveStims_df = (None, None)
+        else:
+            # If we don't have task replay then we set the treplay intervals to NaN in our passivePeriods_df dataset
             passiveGabor_df, passiveStims_df = (None, None)
+            passivePeriods_df.taskReplay = np.NAN
 
         if plot:
             f, ax = plt.subplots(1, 1)
diff --git a/ibllib/pipes/behavior_tasks.py b/ibllib/pipes/behavior_tasks.py
index b4105affa..0a677963a 100644
--- a/ibllib/pipes/behavior_tasks.py
+++ b/ibllib/pipes/behavior_tasks.py
@@ -208,10 +208,10 @@ def signature(self):
                             (f'_{self.sync_namespace}_sync.times.*', self.sync_collection, True),
                             ('*.wiring.json', self.sync_collection, False),
                             ('*.meta', self.sync_collection, False)],
-            'output_files': [('_ibl_passiveGabor.table.csv', self.output_collection, True),
+            'output_files': [('_ibl_passiveGabor.table.csv', self.output_collection, False),
                              ('_ibl_passivePeriods.intervalsTable.csv', self.output_collection, True),
                              ('_ibl_passiveRFM.times.npy', self.output_collection, True),
-                             ('_ibl_passiveStims.table.csv', self.output_collection, True)]
+                             ('_ibl_passiveStims.table.csv', self.output_collection, False)]
         }
         return signature
 
@@ -240,10 +240,10 @@ def signature(self):
                             (f'_{self.sync_namespace}_sync.channels.*', self.sync_collection, False),
                             (f'_{self.sync_namespace}_sync.polarities.*', self.sync_collection, False),
                             (f'_{self.sync_namespace}_sync.times.*', self.sync_collection, False)],
-            'output_files': [('_ibl_passiveGabor.table.csv', self.output_collection, True),
+            'output_files': [('_ibl_passiveGabor.table.csv', self.output_collection, False),
                              ('_ibl_passivePeriods.intervalsTable.csv', self.output_collection, True),
                              ('_ibl_passiveRFM.times.npy', self.output_collection, True),
-                             ('_ibl_passiveStims.table.csv', self.output_collection, True)]
+                             ('_ibl_passiveStims.table.csv', self.output_collection, False)]
         }
         return signature
 

From 7e09fe2e2e89e9a6088a600c75c18fda5aa7ff5b Mon Sep 17 00:00:00 2001
From: Miles Wells <k1o0@5tk.co>
Date: Tue, 25 Jun 2024 16:25:56 +0300
Subject: [PATCH 10/15] Deprecate old SpikeSorting task

---
 ibllib/pipes/ephys_preprocessing.py | 14 +++++--
 ibllib/tests/test_pipes.py          | 59 +++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/ibllib/pipes/ephys_preprocessing.py b/ibllib/pipes/ephys_preprocessing.py
index d13906e15..3855fd921 100644
--- a/ibllib/pipes/ephys_preprocessing.py
+++ b/ibllib/pipes/ephys_preprocessing.py
@@ -39,8 +39,8 @@
 from ibllib.plots.snapshot import ReportSnapshot
 from brainbox.behavior.dlc import likelihood_threshold, get_licks, get_pupil_diameter, get_smooth_pupil_diameter
 
-_logger = logging.getLogger("ibllib")
-warnings.warn('`pipes.training_preprocessing` to be removed in favour of dynamic pipeline')
+_logger = logging.getLogger('ibllib')
+warnings.warn('`pipes.ephys_preprocessing` to be removed in favour of dynamic pipeline', DeprecationWarning)
 
 
 #  level 0
@@ -53,7 +53,7 @@ class EphysPulses(tasks.Task):
     io_charge = 30  # this jobs reads raw ap files
     priority = 90  # a lot of jobs depend on this one
     level = 0  # this job doesn't depend on anything
-    force = False  # whether or not to force download of missing data on local server if outputs already exist
+    force = False  # whether to force download of missing data on local server if outputs already exist
     signature = {
         'input_files': [('*ap.meta', 'raw_ephys_data/probe*', True),
                         ('*ap.ch', 'raw_ephys_data/probe*', False),  # not necessary when we have .bin file
@@ -219,7 +219,7 @@ def _run(self, overwrite=False):
 
 class SpikeSorting(tasks.Task):
     """
-    Pykilosort 2.5 pipeline
+    (DEPRECATED) Pykilosort 2.5 pipeline
     """
     gpu = 1
     io_charge = 100  # this jobs reads raw ap files
@@ -238,6 +238,12 @@ class SpikeSorting(tasks.Task):
         'output_files': []  # see setUp method for declaration of inputs
     }
 
+    def __init__(self, *args, **kwargs):
+        warnings.warn('`pipes.ephys_preprocessing.SpikeSorting` to be removed '
+                      'in favour of `pipes.ephys_tasks.SpikeSorting`',
+                      DeprecationWarning)
+        super().__init__(*args, **kwargs)
+
     @staticmethod
     def spike_sorting_signature(pname=None):
         pname = pname if pname is not None else "probe*"
diff --git a/ibllib/tests/test_pipes.py b/ibllib/tests/test_pipes.py
index 491036917..14d0643af 100644
--- a/ibllib/tests/test_pipes.py
+++ b/ibllib/tests/test_pipes.py
@@ -795,5 +795,64 @@ def dummy_function(arg1, arg2):
         self.assertEqual(result, ("test1", "test2"))
 
 
+class TestLegacyDeprecations(unittest.TestCase):
+    """Assert removal of old code."""
+
+    def test_remove_legacy_pipeline(self):
+        """Remove old legacy pipeline code.
+
+        The following code is an incomplete list of modules and functions that should be removed:
+
+        - pipes.ephys_preprocessing
+        - pipes.training_preprocessing
+        - io.extractors.biased_trials.extract_all
+        - io.extractors.bpod_trials.extract_all
+        - io.extractors.base.get_session_extractor_type
+        - io.extractors.base.get_pipeline
+        - io.extractors.base._get_pipeline_from_task_type
+        - io.extractors.base._get_task_types_json_config
+        - io.extractors.extractor_types.json
+        - qc.task_extractors.TaskQCExtractor.extract_data
+
+        NB: some tasks in ephys_preprocessing and maybe training_preprocessing may be directly used
+        or subclassed by the dynamic pipeline. The TaskQCExtractor class could be removed entirely.
+        Instead, a function could exist to simply fetch the relevant data from the task's extractor
+        class.  Alos, there may be plenty of iblscripts CI tests to be removed.
+        """
+        self.assertTrue(datetime.today() < datetime(2024, 9, 1), 'remove legacy pipeline')
+
+    def test_remove_legacy_rig_code(self):
+        """Remove old legacy (v7) rig code.
+
+        The following code is an incomplete list of modules and functions that should be removed:
+
+        - pipes.transfer_rig_data
+        - pipes.misc.check_transfer
+        - pipes.misc.transfer_session_folders
+        - pipes.misc.copy_with_check
+        - pipes.misc.backup_session
+        - pipes.misc.transfer_folder
+        - pipes.misc.load_videopc_params
+        - pipes.misc.load_ephyspc_params
+        - pipes.misc.create_basic_transfer_params
+        - pipes.misc.create_videopc_params
+        - pipes.misc.create_ephyspc_params
+        - pipes.misc.rdiff_install
+        - pipes.misc.rsync_paths
+        - pipes.misc.confirm_ephys_remote_folder
+        - pipes.misc.create_ephys_flags
+        - pipes.misc.create_ephys_transfer_done_flag
+        - pipes.misc.create_video_transfer_done_flag
+        - pipes.misc.create_transfer_done_flag
+
+        pipes.misc.backup_session may be worth keeping and utilized by the iblrig code (arguably
+        useful on both rig and local server). The corresponding tests should also be removed.
+
+        In addition some iblscripts.deploy files should be removed, e.g. prepare_ephys_session,
+        prepare_video_session.
+        """
+        self.assertTrue(datetime.today() < datetime(2024, 10, 1), 'remove legacy rig code')
+
+
 if __name__ == '__main__':
     unittest.main(exit=False, verbosity=2)

From dc913f5f3c6f573c2e2e3af3f8dcefcb0ab96a98 Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Wed, 26 Jun 2024 12:09:32 +0100
Subject: [PATCH 11/15] for v8 ephys sessions use TrialsTableBiased

---
 ibllib/io/extractors/biased_trials.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ibllib/io/extractors/biased_trials.py b/ibllib/io/extractors/biased_trials.py
index 3be854179..b11a989b4 100644
--- a/ibllib/io/extractors/biased_trials.py
+++ b/ibllib/io/extractors/biased_trials.py
@@ -185,8 +185,17 @@ class EphysTrials(BaseBpodTrialsExtractor):
 
     def _extract(self, extractor_classes=None, **kwargs) -> dict:
         extractor_classes = extractor_classes or []
+
+        # For iblrig v8 we use the biased trials table instead. ContrastLeft, ContrastRight and ProbabilityLeft are
+        # filled from the values in the bpod data itself rather than using the pregenerated session number
+        iblrig_version = self.settings.get('IBLRIG_VERSION', self.settings.get('IBLRIG_VERSION_TAG', '0'))
+        if version.parse(iblrig_version) >= version.parse('8.0.0'):
+            TrialsTable = TrialsTableBiased
+        else:
+            TrialsTable = TrialsTableEphys
+
         base = [GoCueTriggerTimes, StimOnTriggerTimes, ItiInTimes, StimOffTriggerTimes, StimFreezeTriggerTimes,
-                ErrorCueTriggerTimes, TrialsTableEphys, IncludedTrials, PhasePosQuiescence]
+                ErrorCueTriggerTimes, TrialsTable, IncludedTrials, PhasePosQuiescence]
         # Get all detected TTLs. These are stored for QC purposes
         self.frame2ttl, self.audio = raw.load_bpod_fronts(self.session_path, data=self.bpod_trials)
         # Exclude from trials table

From 18bbf8934789a4906ba679de1042b8ecf1aaef9d Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Wed, 26 Jun 2024 14:24:33 +0100
Subject: [PATCH 12/15] pass in correct collection to behavior plots

---
 ibllib/plots/figures.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ibllib/plots/figures.py b/ibllib/plots/figures.py
index f51666773..384042add 100644
--- a/ibllib/plots/figures.py
+++ b/ibllib/plots/figures.py
@@ -107,7 +107,7 @@ def __init__(self, eid, session_path=None, one=None, **kwargs):
         self.one = one
         self.eid = eid
         self.session_path = session_path or self.one.eid2path(self.eid)
-        self.trials_collection = kwargs.pop('trials_collection', 'alf')
+        self.trials_collection = kwargs.pop('task_collection', 'alf')
         super(BehaviourPlots, self).__init__(self.session_path, self.eid, one=self.one,
                                              **kwargs)
         # Output directory should mirror trials collection, sans 'alf' part

From 089274b179fbadba623364da16c36a633be7b4b8 Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Thu, 27 Jun 2024 09:18:34 +0100
Subject: [PATCH 13/15] release notes and version

---
 ibllib/__init__.py | 2 +-
 release_notes.md   | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ibllib/__init__.py b/ibllib/__init__.py
index 8f7a2aca5..6bbf9a45d 100644
--- a/ibllib/__init__.py
+++ b/ibllib/__init__.py
@@ -2,7 +2,7 @@
 import logging
 import warnings
 
-__version__ = '2.36.0'
+__version__ = '2.37.0'
 warnings.filterwarnings('always', category=DeprecationWarning, module='ibllib')
 
 # if this becomes a full-blown library we should let the logging configuration to the discretion of the dev
diff --git a/release_notes.md b/release_notes.md
index 07e473596..36cfb22c1 100644
--- a/release_notes.md
+++ b/release_notes.md
@@ -1,3 +1,11 @@
+## Release Note 2.37.0
+
+### features
+- Add code in preparation for retirement of old training and ephys local server pipelines
+
+### bugfixes
+- Change handling of trials extraction of iblrigv8 sessions such that pregenerated session is not used
+
 ## Release Note 2.36.0
 
 ### features

From 7c2135cfbe0923a5c227a680fbd657c496943f6f Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Thu, 27 Jun 2024 11:11:21 +0100
Subject: [PATCH 14/15] fix to check for incorrect namespace when sync is nidq

---
 ibllib/pipes/dynamic_pipeline.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ibllib/pipes/dynamic_pipeline.py b/ibllib/pipes/dynamic_pipeline.py
index 9abce0a46..17c0b2bc4 100644
--- a/ibllib/pipes/dynamic_pipeline.py
+++ b/ibllib/pipes/dynamic_pipeline.py
@@ -162,9 +162,8 @@ def _sync_label(sync, acquisition_software=None, **_):
     str
         The sync label for determining the extractor tasks.
     """
-    if sync == 'nidq' and acquisition_software == 'timeline':
-        return 'timeline'
-    return sync
+
+    return acquisition_software if (sync == 'nidq' and acquisition_software != 'spikeglx') else sync
 
 
 def make_pipeline(session_path, **pkwargs):
@@ -279,7 +278,10 @@ def make_pipeline(session_path, **pkwargs):
             # -   choice_world_habituation
             if 'passiveChoiceWorld' in protocol:
                 registration_class = btasks.PassiveRegisterRaw
-                behaviour_class = getattr(btasks, 'PassiveTask' + sync_label.capitalize())
+                try:
+                    behaviour_class = getattr(btasks, 'PassiveTask' + sync_label.capitalize())
+                except AttributeError:
+                    raise NotImplementedError(f'No passive task available for sync namespace "{sync_label}"')
                 compute_status = False
             elif 'habituation' in protocol:
                 registration_class = btasks.HabituationRegisterRaw
@@ -287,7 +289,10 @@ def make_pipeline(session_path, **pkwargs):
                 compute_status = False
             else:
                 registration_class = btasks.TrialRegisterRaw
-                behaviour_class = getattr(btasks, 'ChoiceWorldTrials' + sync_label.capitalize())
+                try:
+                    behaviour_class = getattr(btasks, 'ChoiceWorldTrials' + sync_label.capitalize())
+                except AttributeError:
+                    raise NotImplementedError(f'No trials task available for sync namespace "{sync_label}"')
                 compute_status = True
             tasks[f'RegisterRaw_{protocol}_{i:02}'] = type(f'RegisterRaw_{protocol}_{i:02}', (registration_class,), {})(
                 **kwargs, **task_kwargs)

From 780126b6880f2965d711af295165efedd44e459f Mon Sep 17 00:00:00 2001
From: Mayo Faulkner <mayo.faulkner@ucl.ac.uk>
Date: Thu, 27 Jun 2024 11:43:17 +0100
Subject: [PATCH 15/15] account for no acquisition software

---
 ibllib/pipes/dynamic_pipeline.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ibllib/pipes/dynamic_pipeline.py b/ibllib/pipes/dynamic_pipeline.py
index 17c0b2bc4..a4f2d5735 100644
--- a/ibllib/pipes/dynamic_pipeline.py
+++ b/ibllib/pipes/dynamic_pipeline.py
@@ -163,7 +163,7 @@ def _sync_label(sync, acquisition_software=None, **_):
         The sync label for determining the extractor tasks.
     """
 
-    return acquisition_software if (sync == 'nidq' and acquisition_software != 'spikeglx') else sync
+    return acquisition_software if (sync == 'nidq' and acquisition_software not in ('spikeglx', None)) else sync
 
 
 def make_pipeline(session_path, **pkwargs):