Merge branch 'develop' into multiEnv

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:

ibllib/tests/test_base_tasks.py

@@ -36,24 +36,25 @@ 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):
         """Test ibllib.pipes.base_tasks.RegisterRawDataTask.register_snapshots.
 
         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()
         self.assertEqual(2, len(notes))
         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'

ibllib/tests/test_pipes.py

@@ -699,6 +699,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,8 +717,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=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)
@@ -728,7 +731,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)