Fix SingleImageVideo caching (#1330)

* Set `SingleImageVideo.caching` as a class attribute

* Modify tests for `SingleImageVideo.caching`

* Add caching as a preference and menu checkbox

* Test `SingleImageVideo.toggle_caching`

* Remove GUI elements for `SingleImageVideo.CACHING`

* Remove remaining prefs for `SingleImageVideo.CACHING`

* Add depreciated comment

* Clean-up

* Update comments

sleap/gui/app.py

@@ -161,6 +161,7 @@ def __init__(
             self.state["share usage data"] = False
         self.state["clipboard_track"] = None
         self.state["clipboard_instance"] = None
+
         self.state.connect("marker size", self.plotFrame)
         self.state.connect("node label size", self.plotFrame)
         self.state.connect("show non-visible nodes", self.plotFrame)

sleap/io/video.py

@@ -809,14 +809,14 @@ class SingleImageVideo:
     """
 
     EXTS = ("jpg", "jpeg", "png", "tif", "tiff")
+    CACHING = False  # Deprecated, but keeping functionality for now.
 
     filename: Optional[str] = attr.ib(default=None)
     filenames: Optional[List[str]] = attr.ib(factory=list)
     height_: Optional[int] = attr.ib(default=None)
     width_: Optional[int] = attr.ib(default=None)
     channels_: Optional[int] = attr.ib(default=None)
     grayscale: Optional[bool] = attr.ib()
-    caching: bool = attr.ib(default=False)
 
     _detect_grayscale = False
 
@@ -871,7 +871,7 @@ def _load_test_frame(self):
                 self.width_ = test_frame_.shape[1]
             if self.channels_ is None:
                 self.channels_ = test_frame_.shape[2]
-        if self.caching:
+        if self.CACHING:  # Deprecated, but keeping functionality for now.
             self.test_frame_ = test_frame_
         return test_frame_
 
@@ -980,7 +980,7 @@ def reset(
 
     def get_frame(self, idx: int, grayscale: bool = None) -> np.ndarray:
         """See :class:`Video`."""
-        if self.caching:
+        if self.CACHING:  # Deprecated, but keeping functionality for now.
             if idx not in self.cache_:
                 self.cache_[idx] = self._load_idx(idx)
 

tests/fixtures/datasets.py

@@ -20,6 +20,7 @@
 TEST_MAT_LABELS = "tests/data/mat/labels.mat"
 TEST_SLP_MIN_LABELS_ROBOT = "tests/data/slp_hdf5/small_robot_minimal.slp"
 TEST_SLP_SIV_ROBOT = "tests/data/siv_format_v1/small_robot_siv.slp"
+TEST_SLP_SIV_ROBOT_CACHING = "tests/data/siv_format_v2/small_robot_siv_caching.slp"
 TEST_MIN_TRACKS_2NODE_LABELS = "tests/data/tracks/clip.2node.slp"
 TEST_MIN_TRACKS_13NODE_LABELS = "tests/data/tracks/clip.slp"
 TEST_HDF5_PREDICTIONS = "tests/data/hdf5_format_v1/centered_pair_predictions.h5"
@@ -63,6 +64,22 @@ def siv_robot():
     return Labels.load_file(TEST_SLP_SIV_ROBOT, video_search="tests/data/videos/")
 
 
+@pytest.fixture
+def siv_robot_caching():
+    """Created after caching attribute was added to `SingleImageVideo` backend.
+
+    The typehinting of the `caching` attribute (#1243) caused it to be used by cattrs to
+    determine which type of dataclass to use. However, the older datasets containing
+    `SingleImageVideo`s were now being read in as `NumpyVideo`s. Although removing the
+    typehinting from `caching` seems to do the trick (and never made it into an official
+    release), this is a fixture to test that datasets created while `caching` was added
+    into the serialization are read in correctly.
+    """
+    return Labels.load_file(
+        TEST_SLP_SIV_ROBOT_CACHING, video_search="tests/data/videos/"
+    )
+
+
 @pytest.fixture
 def min_tracks_2node_labels():
     return Labels.load_file(

tests/io/test_video.py

@@ -12,7 +12,7 @@
     DummyVideo,
     load_video,
 )
-from tests.fixtures.datasets import TEST_SLP_SIV_ROBOT
+
 from tests.fixtures.videos import (
     TEST_H5_DSET,
     TEST_H5_INPUT_FORMAT,
@@ -495,7 +495,7 @@ def test_reset_video_mp4(small_robot_mp4_vid: Video):
     assert_video_params(video=video, filename=filename, bgr=True, reset=True)
 
 
-def test_reset_video_siv(small_robot_single_image_vid: Video):
+def test_reset_video_siv(small_robot_single_image_vid: Video, siv_robot):
     video = small_robot_single_image_vid
     filename = video.backend.filename
 
@@ -547,19 +547,24 @@ def test_reset_video_siv(small_robot_single_image_vid: Video):
         video.backend.reset(filename=filename, filenames=filenames)
     assert_video_params(video=video, filenames=filenames, reset=True)
 
-    # Test reset does not break deserialization of older slp
-    labels: Labels = Labels.load_file(
-        TEST_SLP_SIV_ROBOT, video_search="tests/data/videos/"
-    )
+    # Test reset does not break deserialization of older slp.
+    labels = siv_robot  # This is actually tested upon passing in the fixture.
     video: Video = labels.video
     filename = labels.video.backend.filename
     labels.video.backend.reset(filename=filename, grayscale=True)
     assert_video_params(video=video, filenames=filenames, grayscale=True, reset=True)
 
 
-def test_singleimagevideo_caching():
+def test_singleimagevideo_caching(siv_robot_caching):
+    # Test that older `SingleImageVideo` with type-hinted `caching` can be read in.
+    siv_robot_caching  # This is actually tested upon passing in the fixture.
+
+    # The below tests are for depreciated `SingleImageVideo.CACHING` functionality.
+
     # With caching
-    video = Video.from_filename(TEST_SMALL_ROBOT_SIV_FILE0, caching=True)
+    filename = siv_robot_caching.video.backend.filename
+    video = Video.from_filename(filename)
+    SingleImageVideo.CACHING = True
     assert video.backend.test_frame_ is None
     assert len(video.backend.cache_) == 0
     assert video.backend.channels_ is None
@@ -573,7 +578,8 @@ def test_singleimagevideo_caching():
     assert len(video.backend.cache_) == 1  # Loaded frame stored!
 
     # No caching
-    video = Video.from_filename(TEST_SMALL_ROBOT_SIV_FILE0, caching=False)
+    video = Video.from_filename(filename)
+    SingleImageVideo.CACHING = False
     assert video.backend.test_frame_ is None
     assert len(video.backend.cache_) == 0
     assert video.backend.channels_ is None