From f314fe69b273da7fbef2676820bb875446106421 Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 16:29:08 -0700 Subject: [PATCH 1/8] Safer video loading from SLP --- sleap_io/io/main.py | 7 +++- sleap_io/io/slp.py | 91 +++++++++++++++++++++++++++------------------ 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/sleap_io/io/main.py b/sleap_io/io/main.py index 7fd702f7..c72017ea 100644 --- a/sleap_io/io/main.py +++ b/sleap_io/io/main.py @@ -7,16 +7,19 @@ from pathlib import Path -def load_slp(filename: str) -> Labels: +def load_slp(filename: str, open_videos: bool = True) -> Labels: """Load a SLEAP dataset. Args: filename: Path to a SLEAP labels file (`.slp`). + open_videos: If `True` (the default), attempt to open the video backend for + I/O. If `False`, the backend will not be opened (useful for reading metadata + when the video files are not available). Returns: The dataset as a `Labels` object. """ - return slp.read_labels(filename) + return slp.read_labels(filename, open_videos=open_videos) def save_slp( diff --git a/sleap_io/io/slp.py b/sleap_io/io/slp.py index 605545aa..262ee3d0 100644 --- a/sleap_io/io/slp.py +++ b/sleap_io/io/slp.py @@ -44,7 +44,10 @@ class InstanceType(IntEnum): def make_video( - labels_path: str, video_json: dict, video_ind: int | None = None + labels_path: str, + video_json: dict, + video_ind: int | None = None, + open_backend: bool = True, ) -> Video: """Create a `Video` object from a JSON dictionary. @@ -53,6 +56,9 @@ def make_video( video_json: A dictionary containing the video metadata. video_ind: The index of the video in the labels file. This is used to try to recover the source video for embedded videos. This is skipped if `None`. + open_backend: If `True` (the default), attempt to open the video backend for + I/O. If `False`, the backend will not be opened (useful for reading metadata + when the video files are not available). """ backend_metadata = video_json["backend"] video_path = backend_metadata["filename"] @@ -67,22 +73,6 @@ def make_video( # Basic path resolution. video_path = Path(Path(video_path).as_posix().replace("\\", "/")) - try: - if not video_path.exists(): - # Check for the same filename in the same directory as the labels file. - video_path_ = Path(labels_path).parent / video_path.name - if video_path_.exists(): - video_path = video_path_ - else: - # TODO (TP): Expand capabilities of path resolution to support more - # complex path finding strategies. - pass - except OSError: - pass - - # Convert video path to string. - video_path = video_path.as_posix() - if is_embedded: # Try to recover the source video. with h5py.File(labels_path, "r") as f: @@ -91,23 +81,44 @@ def make_video( f[f"video{video_ind}/source_video"].attrs["json"] ) source_video = make_video( - labels_path, source_video_json, video_ind=None + labels_path, + source_video_json, + video_ind=None, + open_backend=open_backend, ) - if "filenames" in backend_metadata: - # This is an ImageVideo. - # TODO: Path resolution. - video_path = backend_metadata["filenames"] - - try: - backend = VideoBackend.from_filename( - video_path, - dataset=backend_metadata.get("dataset", None), - grayscale=backend_metadata.get("grayscale", None), - input_format=backend_metadata.get("input_format", None), - ) - except ValueError: - backend = None + backend = None + if open_backend: + try: + if not video_path.exists(): + # Check for the same filename in the same directory as the labels file. + video_path_ = Path(labels_path).parent / video_path.name + if video_path_.exists() and video_path.stat(): + video_path = video_path_ + else: + # TODO (TP): Expand capabilities of path resolution to support more + # complex path finding strategies. + pass + except (OSError, PermissionError, FileNotFoundError): + pass + + # Convert video path to string. + video_path = video_path.as_posix() + + if "filenames" in backend_metadata: + # This is an ImageVideo. + # TODO: Path resolution. + video_path = backend_metadata["filenames"] + + try: + backend = VideoBackend.from_filename( + video_path, + dataset=backend_metadata.get("dataset", None), + grayscale=backend_metadata.get("grayscale", None), + input_format=backend_metadata.get("input_format", None), + ) + except: + backend = None return Video( filename=video_path, @@ -117,11 +128,14 @@ def make_video( ) -def read_videos(labels_path: str) -> list[Video]: +def read_videos(labels_path: str, open_backend: bool = False) -> list[Video]: """Read `Video` dataset in a SLEAP labels file. Args: labels_path: A string path to the SLEAP labels file. + open_backend: If `True` (the default), attempt to open the video backend for + I/O. If `False`, the backend will not be opened (useful for reading metadata + when the video files are not available). Returns: A list of `Video` objects. @@ -131,7 +145,9 @@ def read_videos(labels_path: str) -> list[Video]: read_hdf5_dataset(labels_path, "videos_json") ): video_json = json.loads(video_data) - video = make_video(labels_path, video_json, video_ind=video_ind) + video = make_video( + labels_path, video_json, video_ind=video_ind, open_backend=open_backend + ) videos.append(video) return videos @@ -1003,17 +1019,20 @@ def write_lfs(labels_path: str, labels: Labels): ) -def read_labels(labels_path: str) -> Labels: +def read_labels(labels_path: str, open_videos: bool = True) -> Labels: """Read a SLEAP labels file. Args: labels_path: A string path to the SLEAP labels file. + open_videos: If `True` (the default), attempt to open the video backend for + I/O. If `False`, the backend will not be opened (useful for reading metadata + when the video files are not available). Returns: The processed `Labels` object. """ tracks = read_tracks(labels_path) - videos = read_videos(labels_path) + videos = read_videos(labels_path, open_backend=open_videos) skeletons = read_skeletons(labels_path) points = read_points(labels_path) pred_points = read_pred_points(labels_path) From 8c5bac3176ce145c5497c5b2bb6bf916702801a5 Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 16:47:29 -0700 Subject: [PATCH 2/8] Add explicit control of video backend opening --- sleap_io/io/slp.py | 8 ++++++-- sleap_io/model/video.py | 22 ++++++++++++++++++++-- tests/io/test_slp.py | 9 +++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/sleap_io/io/slp.py b/sleap_io/io/slp.py index 262ee3d0..208c6fc4 100644 --- a/sleap_io/io/slp.py +++ b/sleap_io/io/slp.py @@ -109,6 +109,9 @@ def make_video( # This is an ImageVideo. # TODO: Path resolution. video_path = backend_metadata["filenames"] + video_path = [ + Path(Path(p).as_posix().replace("\\", "/")) for p in video_path + ] try: backend = VideoBackend.from_filename( @@ -117,7 +120,7 @@ def make_video( grayscale=backend_metadata.get("grayscale", None), input_format=backend_metadata.get("input_format", None), ) - except: + except Exception: backend = None return Video( @@ -125,10 +128,11 @@ def make_video( backend=backend, backend_metadata=backend_metadata, source_video=source_video, + open_backend=open_backend, ) -def read_videos(labels_path: str, open_backend: bool = False) -> list[Video]: +def read_videos(labels_path: str, open_backend: bool = True) -> list[Video]: """Read `Video` dataset in a SLEAP labels file. Args: diff --git a/sleap_io/model/video.py b/sleap_io/model/video.py index edd49489..8239cbb6 100644 --- a/sleap_io/model/video.py +++ b/sleap_io/model/video.py @@ -34,6 +34,11 @@ class Video: information) without having access to the video file itself. source_video: The source video object if this is a proxy video. This is present when the video contains an embedded subset of frames from another video. + open_backend: Whether to open the backend when the video is available. If `True` + (the default), the backend will be automatically opened if the video exists. + Set this to `False` when you want to manually open the backend, or when the + you know the video file does not exist and you want to avoid trying to open + the file. Notes: Instances of this class are hashed by identity, not by value. This means that @@ -47,12 +52,13 @@ class Video: backend: Optional[VideoBackend] = None backend_metadata: dict[str, any] = attrs.field(factory=dict) source_video: Optional[Video] = None + open_backend: bool = True EXTS = MediaVideo.EXTS + HDF5Video.EXTS + ImageVideo.EXTS def __attrs_post_init__(self): """Post init syntactic sugar.""" - if self.backend is None and self.exists(): + if self.open_backend and self.backend is None and self.exists(): self.open() @classmethod @@ -181,7 +187,13 @@ def __getitem__(self, inds: int | list[int] | slice) -> np.ndarray: See also: VideoBackend.get_frame, VideoBackend.get_frames """ if not self.is_open: - self.open() + if self.open_backend: + self.open() + else: + raise ValueError( + "Video backend is not open. Call video.open() or set " + "video.open_backend to True to do automatically on frame read." + ) return self.backend[inds] def exists(self, check_all: bool = False) -> bool: @@ -208,6 +220,7 @@ def is_open(self) -> bool: def open( self, + filename: Optional[str] = None, dataset: Optional[str] = None, grayscale: Optional[str] = None, keep_open: bool = True, @@ -215,6 +228,8 @@ def open( """Open the video backend for reading. Args: + filename: Filename to open. If not specified, will use the filename set on + the video object. dataset: Name of dataset in HDF5 file. grayscale: Whether to force grayscale. If None, autodetect on first frame load. @@ -231,6 +246,9 @@ def open( Values for the HDF5 dataset and grayscale will be remembered if not specified. """ + if self.filename: + self.replace_filename(filename, open=False) + if not self.exists(): raise FileNotFoundError(f"Video file not found: {self.filename}") diff --git a/tests/io/test_slp.py b/tests/io/test_slp.py index cd37b9a1..18c7d2ed 100644 --- a/tests/io/test_slp.py +++ b/tests/io/test_slp.py @@ -354,3 +354,12 @@ def test_embed_two_rounds(tmpdir, slp_real_data): == "tests/data/videos/centered_pair_low_quality.mp4" ) assert type(labels3.video.backend) == MediaVideo + + +def test_lazy_video_read(slp_real_data): + labels = read_labels(slp_real_data) + assert type(labels.video.backend) == MediaVideo + assert labels.video.exists() + + labels = read_labels(slp_real_data, open_videos=False) + assert labels.video.backend is None From d04fea5f395924c41ffc8b128ed465492ab83b57 Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 17:05:09 -0700 Subject: [PATCH 3/8] Fix tests --- sleap_io/io/slp.py | 40 ++++++++++++++++++++++++++++------------ sleap_io/model/video.py | 2 +- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/sleap_io/io/slp.py b/sleap_io/io/slp.py index 208c6fc4..93b42dc0 100644 --- a/sleap_io/io/slp.py +++ b/sleap_io/io/slp.py @@ -43,6 +43,23 @@ class InstanceType(IntEnum): PREDICTED = 1 +def sanitize_filename( + filename: str | Path | list[str] | list[Path], +) -> str | list[str]: + """Sanitize a filename to a canonical posix-compatible format. + + Args: + filename: A string or `Path` object or list of either to sanitize. + + Returns: + A sanitized filename as a string (or list of strings if a list was provided) + with forward slashes and posix-formatted. + """ + if isinstance(filename, list): + return [sanitize_filename(f) for f in filename] + return Path(filename).as_posix().replace("\\", "/") + + def make_video( labels_path: str, video_json: dict, @@ -71,7 +88,7 @@ def make_video( is_embedded = True # Basic path resolution. - video_path = Path(Path(video_path).as_posix().replace("\\", "/")) + video_path = Path(sanitize_filename(video_path)) if is_embedded: # Try to recover the source video. @@ -109,9 +126,7 @@ def make_video( # This is an ImageVideo. # TODO: Path resolution. video_path = backend_metadata["filenames"] - video_path = [ - Path(Path(p).as_posix().replace("\\", "/")) for p in video_path - ] + video_path = [Path(sanitize_filename(p)) for p in video_path] try: backend = VideoBackend.from_filename( @@ -165,16 +180,17 @@ def video_to_dict(video: Video) -> dict: Returns: A dictionary containing the video metadata. """ + video_filename = sanitize_filename(video.filename) if video.backend is None: - return {"filename": video.filename, "backend": video.backend_metadata} + return {"filename": video_filename, "backend": video.backend_metadata} if type(video.backend) == MediaVideo: return { - "filename": video.filename, + "filename": video_filename, "backend": { "type": "MediaVideo", "shape": video.shape, - "filename": video.filename, + "filename": video_filename, "grayscale": video.grayscale, "bgr": True, "dataset": "", @@ -184,12 +200,12 @@ def video_to_dict(video: Video) -> dict: elif type(video.backend) == HDF5Video: return { - "filename": video.filename, + "filename": video_filename, "backend": { "type": "HDF5Video", "shape": video.shape, "filename": ( - "." if video.backend.has_embedded_images else video.filename + "." if video.backend.has_embedded_images else video_filename ), "dataset": video.backend.dataset, "input_format": video.backend.input_format, @@ -200,12 +216,12 @@ def video_to_dict(video: Video) -> dict: elif type(video.backend) == ImageVideo: return { - "filename": video.filename, + "filename": video_filename, "backend": { "type": "ImageVideo", "shape": video.shape, - "filename": video.backend.filename[0], - "filenames": video.backend.filename, + "filename": sanitize_filename(video.backend.filename[0]), + "filenames": sanitize_filename(video.backend.filename), "dataset": video.backend_metadata.get("dataset", None), "grayscale": video.grayscale, "input_format": video.backend_metadata.get("input_format", None), diff --git a/sleap_io/model/video.py b/sleap_io/model/video.py index 8239cbb6..70a65e58 100644 --- a/sleap_io/model/video.py +++ b/sleap_io/model/video.py @@ -246,7 +246,7 @@ def open( Values for the HDF5 dataset and grayscale will be remembered if not specified. """ - if self.filename: + if filename is not None: self.replace_filename(filename, open=False) if not self.exists(): From b0af70f0c0d5bfa998d13c9faa5d6a610a7cf38e Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 17:09:18 -0700 Subject: [PATCH 4/8] Docs note --- docs/index.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/index.md b/docs/index.md index ea3af789..41922c1d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -134,14 +134,16 @@ labels.save("labels.slp") ```py import sleap_io as sio -labels = sio.load_file("labels.v001.slp") +# Load labels without trying to open the video files. +labels = sio.load_file("labels.v001.slp", open_videos=False) -# Fix paths using prefixes. +# Fix paths using prefix replacement. labels.replace_filenames(prefix_map={ "D:/data/sleap_projects": "/home/user/sleap_projects", "C:/Users/sleaper/Desktop/test": "/home/user/sleap_projects", }) +# Save labels with updated paths. labels.save("labels.v002.slp") ``` From 8e179cac60c8ed652e1788177aa2510b4fccce02 Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 18:21:19 -0700 Subject: [PATCH 5/8] Add explicit check for file accessibility --- sleap_io/io/nwb.py | 70 +++++++++++++++++++++++++++++++- sleap_io/io/slp.py | 13 +++--- sleap_io/io/utils.py | 84 ++++++++------------------------------- sleap_io/model/video.py | 12 ++++-- tests/io/test_slp.py | 36 ++++++++++++++++- tests/model/test_video.py | 23 ++++++++++- 6 files changed, 156 insertions(+), 82 deletions(-) diff --git a/sleap_io/io/nwb.py b/sleap_io/io/nwb.py index 9314294a..b46dc23a 100644 --- a/sleap_io/io/nwb.py +++ b/sleap_io/io/nwb.py @@ -26,7 +26,75 @@ Instance, PredictedInstance, ) -from sleap_io.io.utils import convert_predictions_to_dataframe + + +def convert_predictions_to_dataframe(labels: Labels) -> pd.DataFrame: + """Convert predictions data to a Pandas dataframe. + + Args: + labels: A general label object. + + Returns: + pd.DataFrame: A pandas data frame with the structured data with + hierarchical columns. The column hierarchy is: + "video_path", + "skeleton_name", + "track_name", + "node_name", + And it is indexed by the frames. + + Raises: + ValueError: If no frames in the label objects contain predicted instances. + """ + # Form pairs of labeled_frames and predicted instances + labeled_frames = labels.labeled_frames + all_frame_instance_tuples = ( + (label_frame, instance) # type: ignore + for label_frame in labeled_frames + for instance in label_frame.predicted_instances + ) + + # Extract the data + data_list = list() + for labeled_frame, instance in all_frame_instance_tuples: + # Traverse the nodes of the instances's skeleton + skeleton = instance.skeleton + for node in skeleton.nodes: + row_dict = dict( + frame_idx=labeled_frame.frame_idx, + x=instance.points[node].x, + y=instance.points[node].y, + score=instance.points[node].score, # type: ignore[attr-defined] + node_name=node.name, + skeleton_name=skeleton.name, + track_name=instance.track.name if instance.track else "untracked", + video_path=labeled_frame.video.filename, + ) + data_list.append(row_dict) + + if not data_list: + raise ValueError("No predicted instances found in labels object") + + labels_df = pd.DataFrame(data_list) + + # Reformat the data with columns for dict-like hierarchical data access. + index = [ + "skeleton_name", + "track_name", + "node_name", + "video_path", + "frame_idx", + ] + + labels_tidy_df = ( + labels_df.set_index(index) + .unstack(level=[0, 1, 2, 3]) + .swaplevel(0, -1, axis=1) # video_path on top while x, y score on bottom + .sort_index(axis=1) # Better format for columns + .sort_index(axis=0) # Sorts by frames + ) + + return labels_tidy_df def get_timestamps(series: PoseEstimationSeries) -> np.ndarray: diff --git a/sleap_io/io/slp.py b/sleap_io/io/slp.py index 93b42dc0..777a3f98 100644 --- a/sleap_io/io/slp.py +++ b/sleap_io/io/slp.py @@ -21,10 +21,7 @@ Labels, ) from sleap_io.io.video import VideoBackend, ImageVideo, MediaVideo, HDF5Video -from sleap_io.io.utils import ( - read_hdf5_attrs, - read_hdf5_dataset, -) +from sleap_io.io.utils import read_hdf5_attrs, read_hdf5_dataset, is_file_accessible from enum import IntEnum from pathlib import Path import imageio.v3 as iio @@ -107,11 +104,11 @@ def make_video( backend = None if open_backend: try: - if not video_path.exists(): + if not is_file_accessible(video_path): # Check for the same filename in the same directory as the labels file. - video_path_ = Path(labels_path).parent / video_path.name - if video_path_.exists() and video_path.stat(): - video_path = video_path_ + candidate_video_path = Path(labels_path).parent / video_path.name + if is_file_accessible(candidate_video_path): + video_path = candidate_video_path else: # TODO (TP): Expand capabilities of path resolution to support more # complex path finding strategies. diff --git a/sleap_io/io/utils.py b/sleap_io/io/utils.py index 7b7b9764..a3394193 100644 --- a/sleap_io/io/utils.py +++ b/sleap_io/io/utils.py @@ -3,9 +3,8 @@ from __future__ import annotations import h5py # type: ignore[import] import numpy as np -import pandas as pd # type: ignore[import] -from typing import Any, Union, Optional, Generator -from sleap_io import Labels, LabeledFrame, PredictedInstance +from typing import Any, Union, Optional +from pathlib import Path def read_hdf5_dataset(filename: str, dataset: str) -> np.ndarray: @@ -175,72 +174,23 @@ def _overwrite_hdf5_attr( _overwrite_hdf5_attr(ds, attr_name, attr_value) -def convert_predictions_to_dataframe(labels: Labels) -> pd.DataFrame: - """Convert predictions data to a Pandas dataframe. +def is_file_accessible(filename: str | Path) -> bool: + """Check if a file is accessible. Args: - labels: A general label object. + filename: Path to a file. Returns: - pd.DataFrame: A pandas data frame with the structured data with - hierarchical columns. The column hierarchy is: - "video_path", - "skeleton_name", - "track_name", - "node_name", - And it is indexed by the frames. - - Raises: - ValueError: If no frames in the label objects contain predicted instances. + `True` if the file is accessible, `False` otherwise. + + Notes: + This checks if the file readable by the current user by reading one byte from + the file. """ - # Form pairs of labeled_frames and predicted instances - labeled_frames = labels.labeled_frames - all_frame_instance_tuples: Generator[ - tuple[LabeledFrame, PredictedInstance], None, None - ] = ( - (label_frame, instance) # type: ignore - for label_frame in labeled_frames - for instance in label_frame.predicted_instances - ) - - # Extract the data - data_list = list() - for labeled_frame, instance in all_frame_instance_tuples: - # Traverse the nodes of the instances's skeleton - skeleton = instance.skeleton - for node in skeleton.nodes: - row_dict = dict( - frame_idx=labeled_frame.frame_idx, - x=instance.points[node].x, - y=instance.points[node].y, - score=instance.points[node].score, # type: ignore[attr-defined] - node_name=node.name, - skeleton_name=skeleton.name, - track_name=instance.track.name if instance.track else "untracked", - video_path=labeled_frame.video.filename, - ) - data_list.append(row_dict) - - if not data_list: - raise ValueError("No predicted instances found in labels object") - - labels_df = pd.DataFrame(data_list) - - # Reformat the data with columns for dict-like hierarchical data access. - index = [ - "skeleton_name", - "track_name", - "node_name", - "video_path", - "frame_idx", - ] - - labels_tidy_df = ( - labels_df.set_index(index) - .unstack(level=[0, 1, 2, 3]) - .swaplevel(0, -1, axis=1) # video_path on top while x, y score on bottom - .sort_index(axis=1) # Better format for columns - .sort_index(axis=0) # Sorts by frames - ) - - return labels_tidy_df + filename = Path(filename) + try: + with open(filename, "rb") as f: + f.read(1) + return True + except (FileNotFoundError, PermissionError, OSError, ValueError): + return False diff --git a/sleap_io/model/video.py b/sleap_io/model/video.py index 70a65e58..d331a479 100644 --- a/sleap_io/model/video.py +++ b/sleap_io/model/video.py @@ -9,6 +9,7 @@ from typing import Tuple, Optional, Optional import numpy as np from sleap_io.io.video import VideoBackend, MediaVideo, HDF5Video, ImageVideo +from sleap_io.io.utils import is_file_accessible from pathlib import Path @@ -197,21 +198,24 @@ def __getitem__(self, inds: int | list[int] | slice) -> np.ndarray: return self.backend[inds] def exists(self, check_all: bool = False) -> bool: - """Check if the video file exists. + """Check if the video file exists and is accessible. Args: check_all: If `True`, check that all filenames in a list exist. If `False` (the default), check that the first filename exists. + + Returns: + `True` if the file exists and is accessible, `False` otherwise. """ if isinstance(self.filename, list): if check_all: for f in self.filename: - if not Path(f).exists(): + if not is_file_accessible(f): return False return True else: - return Path(self.filename[0]).exists() - return Path(self.filename).exists() + return is_file_accessible(self.filename[0]) + return is_file_accessible(self.filename) @property def is_open(self) -> bool: diff --git a/tests/io/test_slp.py b/tests/io/test_slp.py index 18c7d2ed..60b9f8e1 100644 --- a/tests/io/test_slp.py +++ b/tests/io/test_slp.py @@ -38,7 +38,7 @@ import simplejson as json import pytest from pathlib import Path - +import shutil from sleap_io.io.video import ImageVideo, HDF5Video, MediaVideo @@ -363,3 +363,37 @@ def test_lazy_video_read(slp_real_data): labels = read_labels(slp_real_data, open_videos=False) assert labels.video.backend is None + + +def test_video_path_resolution(slp_real_data, tmp_path): + labels = read_labels(slp_real_data) + assert ( + Path(labels.video.filename).as_posix() + == "tests/data/videos/centered_pair_low_quality.mp4" + ) + shutil.copyfile(labels.video.filename, tmp_path / "centered_pair_low_quality.mp4") + labels.video.replace_filename( + "fake/path/to/centered_pair_low_quality.mp4", open=False + ) + labels.save(tmp_path / "labels.slp") + + # Resolve when the same video filename is found in the labels directory. + labels = read_labels(tmp_path / "labels.slp") + assert ( + Path(labels.video.filename).as_posix() + == (tmp_path / "centered_pair_low_quality.mp4").as_posix() + ) + assert labels.video.exists() + + # Make the video file inaccessible. + labels.video.replace_filename("new_fake/path/to/inaccessible.mp4", open=False) + labels.save(tmp_path / "labels2.slp") + shutil.copyfile( + tmp_path / "centered_pair_low_quality.mp4", tmp_path / "inaccessible.mp4" + ) + Path(tmp_path / "inaccessible.mp4").chmod(0o000) + + # Fail to resolve when the video file is inaccessible. + labels = read_labels(tmp_path / "labels2.slp") + assert not labels.video.exists() + assert Path(labels.video.filename).as_posix() == "new_fake/path/to/inaccessible.mp4" diff --git a/tests/model/test_video.py b/tests/model/test_video.py index cc87f5a3..9c5e50e6 100644 --- a/tests/model/test_video.py +++ b/tests/model/test_video.py @@ -56,7 +56,7 @@ def test_video_exists(centered_pair_low_quality_video, centered_pair_frame_paths assert video.exists(check_all=True) is False -def test_video_open_close(centered_pair_low_quality_path): +def test_video_open_close(centered_pair_low_quality_path, centered_pair_frame_paths): video = Video(centered_pair_low_quality_path) assert video.is_open assert type(video.backend) == MediaVideo @@ -91,6 +91,10 @@ def test_video_open_close(centered_pair_low_quality_path): video.open(grayscale=True) assert video.shape == (1100, 384, 384, 1) + video.open(centered_pair_frame_paths) + assert video.shape == (3, 384, 384, 1) + assert type(video.backend) == ImageVideo + def test_video_replace_filename( centered_pair_low_quality_path, centered_pair_frame_paths @@ -142,3 +146,20 @@ def test_grayscale(centered_pair_low_quality_path): video.open() assert video.grayscale == True assert video.shape[-1] == 1 + + +def test_open_backend_preference(centered_pair_low_quality_path): + video = Video(centered_pair_low_quality_path) + assert video.is_open + assert type(video.backend) == MediaVideo + + video = Video(centered_pair_low_quality_path, open_backend=False) + assert video.is_open is False + assert video.backend is None + with pytest.raises(ValueError): + video[0] + + video.open_backend = True + img = video[0] + assert video.is_open + assert type(video.backend) == MediaVideo From e97cfff6aac5c0024e3ff06fdb95a5159706046f Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 18:37:41 -0700 Subject: [PATCH 6/8] Skip chmod test on windows --- sleap_io/model/labels.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sleap_io/model/labels.py b/sleap_io/model/labels.py index 67dea5cf..3328c440 100644 --- a/sleap_io/model/labels.py +++ b/sleap_io/model/labels.py @@ -660,13 +660,19 @@ def make_training_splits( labels.suggestions = [] labels.clean() - # Make splits. + # Make train split. labels_train, labels_rest = labels.split(n_train, seed=seed) + + # Make test split. if n_test is not None: if n_test < 1: n_test = (n_test * len(labels)) / len(labels_rest) labels_test, labels_rest = labels_rest.split(n=n_test, seed=seed) - if n_val is not None: + else: + labels_test = labels_rest + + # Make val split. + if n_val is not None or (isinstance(n_val, float) and n_val == 1.0): if n_val < 1: n_val = (n_val * len(labels)) / len(labels_rest) labels_val, _ = labels_rest.split(n=n_val, seed=seed) From c923ddc185ae33b2cc9fa9e597ce281a3e006e70 Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 18:38:56 -0700 Subject: [PATCH 7/8] Revert "Skip chmod test on windows" This reverts commit e97cfff6aac5c0024e3ff06fdb95a5159706046f. --- sleap_io/model/labels.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sleap_io/model/labels.py b/sleap_io/model/labels.py index 3328c440..67dea5cf 100644 --- a/sleap_io/model/labels.py +++ b/sleap_io/model/labels.py @@ -660,19 +660,13 @@ def make_training_splits( labels.suggestions = [] labels.clean() - # Make train split. + # Make splits. labels_train, labels_rest = labels.split(n_train, seed=seed) - - # Make test split. if n_test is not None: if n_test < 1: n_test = (n_test * len(labels)) / len(labels_rest) labels_test, labels_rest = labels_rest.split(n=n_test, seed=seed) - else: - labels_test = labels_rest - - # Make val split. - if n_val is not None or (isinstance(n_val, float) and n_val == 1.0): + if n_val is not None: if n_val < 1: n_val = (n_val * len(labels)) / len(labels_rest) labels_val, _ = labels_rest.split(n=n_val, seed=seed) From 0487ebcd68a496757fcf7f1d777cc38b0a54336b Mon Sep 17 00:00:00 2001 From: Talmo Pereira Date: Sat, 28 Sep 2024 18:39:33 -0700 Subject: [PATCH 8/8] Skip chmod test on windows --- tests/io/test_slp.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/io/test_slp.py b/tests/io/test_slp.py index 60b9f8e1..47d879b2 100644 --- a/tests/io/test_slp.py +++ b/tests/io/test_slp.py @@ -40,6 +40,7 @@ from pathlib import Path import shutil from sleap_io.io.video import ImageVideo, HDF5Video, MediaVideo +import sys def test_read_labels(slp_typical, slp_simple_skel, slp_minimal): @@ -385,15 +386,19 @@ def test_video_path_resolution(slp_real_data, tmp_path): ) assert labels.video.exists() - # Make the video file inaccessible. - labels.video.replace_filename("new_fake/path/to/inaccessible.mp4", open=False) - labels.save(tmp_path / "labels2.slp") - shutil.copyfile( - tmp_path / "centered_pair_low_quality.mp4", tmp_path / "inaccessible.mp4" - ) - Path(tmp_path / "inaccessible.mp4").chmod(0o000) - - # Fail to resolve when the video file is inaccessible. - labels = read_labels(tmp_path / "labels2.slp") - assert not labels.video.exists() - assert Path(labels.video.filename).as_posix() == "new_fake/path/to/inaccessible.mp4" + if sys.platform != "win32": # Windows does not support chmod. + # Make the video file inaccessible. + labels.video.replace_filename("new_fake/path/to/inaccessible.mp4", open=False) + labels.save(tmp_path / "labels2.slp") + shutil.copyfile( + tmp_path / "centered_pair_low_quality.mp4", tmp_path / "inaccessible.mp4" + ) + Path(tmp_path / "inaccessible.mp4").chmod(0o000) + + # Fail to resolve when the video file is inaccessible. + labels = read_labels(tmp_path / "labels2.slp") + assert not labels.video.exists() + assert ( + Path(labels.video.filename).as_posix() + == "new_fake/path/to/inaccessible.mp4" + )