update list_videos_in_folder#3303
Conversation
…iles if filter `video_type` is set. - Accept files without extension - Default folder searching is kept as is (using valid video extensions)
|
Note: CI seems to be failing due to tf-macos, merging #3292 may potentially help |
…set of extensions.
|
@deruyter92 Great to add deprecations, definitely agree. Do you think a separate PR for that specifically would be useful, or is it more efficient to merge this directly ? |
There was a problem hiding this comment.
Really nice work overall! Definitely much better to have the video loading centralized, and deprecations are nice.
I added quite a few comments that I hope will help future us avoid mistakes and make things easier to use, happy to discuss if there are any concerns.
I will open a PR for the deprecations, I have a small design suggestion to ensure we can greatly extend and automate the system later if needed, while keeping current design and lightweight code.
|
|
||
| def collect_video_paths( | ||
| data_path: str | Path | list[str | Path], | ||
| extensions: str | list[str] | None = None, |
There was a problem hiding this comment.
Here also I'd argue for slightly clearer intent: if None, filtered by support, OK.
Currently [] will also result in default behavior. If we want [] to be "accept no extensions", changes would be needed.
Depends on downstream use.
There was a problem hiding this comment.
The extensions parameter is now updated to yield the following groups of behavior:
- Default
None(default) -> only filter directory contents usingSUPPORTED_VIDEOS, but accept all provided files without filtering""-> same behavior asNone(Required for backward compatibility)
- Only select specified extensions
- Sequence of str, e.g.
(".avi", ".mp4")-> filter both files and directories, collect only paths with ".avi" suffix or ".mp4" suffix - String e.g.
".avi"-> identical to single-length sequence:(".avi")
- Sequence of str, e.g.
- Only select no-extensions
- Empty list / tuple
[]-> only select files without extension
- Empty list / tuple
There was a problem hiding this comment.
My prior behavior just treated all Falsy values, including "" the same as None. But this would only yield option 1 and 2.
Maybe the current update is a bit too complex, but at least it supports all cases.
* Add structured deprecation info and warnings Introduce a DLCDeprecationWarning and a DeprecationInfo pydantic model to standardize deprecation metadata (kind, target, replacement, since, removed_in, renamed params) with parsing and validation of versions. Revamp deprecated and renamed_parameter decorators to build messages from DeprecationInfo, emit DLCDeprecationWarning, attach metadata to wrapped callables (__deprecated_info__, __deprecated_params__), use ParamSpec/TypeVar typing for wrappers, and enforce error when both old and new kwargs are passed. Switch to packaging.version for version parsing. * Use DLCDeprecationWarning and add metadata tests Replace generic DeprecationWarning checks with DLCDeprecationWarning and import packaging.version.Version. Add tests verifying deprecated decorators attach metadata (including since/removed_in parsed as Version), validate invalid version inputs, and ensure removed_in > since. Also add tests for renamed_parameter behavior (conflicting old+new raises, metadata attachment, and invalid since handling) and small docstring/name preservation assertions. * Add packaging as core dep
Summary
The helper
list_videos_in_folderin the PyTorch branch (used for collecting a list of videos for inference) currently always filters the input files by extension: either the defaultSUPPORTED_VIDEOSor custom specifiedvideo_type. When the input paths contain directories, it makes sense to require a list of valid extensions in order to collect video files from it. However, for specified video files, this strict requirement is not always the preferred behavior: for instance this does not allow specifying video files without extension.The current PR changes the contract to
SUPPORTED_VIDEOSor customvideo_type)video_typeis provided, do filter the the video files by the specified valid extensions.solves #3300
Details
PR Status
list_videos_in_folderto auxfun_videos: more centralized instead of pytorch-specificcollect_video_paths, which more accurately reflects function. feedback on naming is welcome!SUPPORTED_VIDEOSby default or with specified extensions filter if set.exclude_patternswhich is defaults to ["_labeled.", "_full."] to match prior DLC outputs.list_videos_in_folderand map to newcollect_video_pathsget_list_of_videosand map to newcollect_video_pathsget_video_listand map to newcollect_video_pathsgrab_files_in_folder,get_camerawise_videos(different implementation)Examples:
New signature
deprecation marker: