Repository: Diaoul/subliminal (python)

Issue:
"Insufficient data to process the guess" doesn't show up without `--debug`
as in the title, when there's not enough data to guess, the error doesn't show up without debug - it just prints `1 error`

Pull request:
Show guessit error without --debug
fixes #1164, #1059

Videos that are not guessed are shown with the verbose flag (`-v`), not only with `--debug`.

Refactor `scan_videos` and add a `GuessingError` exception, a `ValueError` subclass for compatibility reasons.

Patch:
diff --git a/changelog.d/1164.change.rst b/changelog.d/1164.change.rst
new file mode 100644
index 0000000..f99bd5f
--- /dev/null
+++ b/changelog.d/1164.change.rst
@@ -0,0 +1,1 @@
+show "Insufficient data to process the guess" without debug, but with verbose
diff --git a/src/subliminal/cli.py b/src/subliminal/cli.py
index 4431c8a..80fc3bd 100644
--- a/src/subliminal/cli.py
+++ b/src/subliminal/cli.py
@@ -36,10 +36,15 @@ from subliminal import (
     refiner_manager,
     region,
     save_subtitles,
-    scan_video,
-    scan_videos,
 )
-from subliminal.core import ARCHIVE_EXTENSIONS, scan_name, search_external_subtitles
+from subliminal.core import (
+    ARCHIVE_EXTENSIONS,
+    collect_video_filepaths,
+    scan_name,
+    scan_video_or_archive,
+    search_external_subtitles,
+)
+from subliminal.exceptions import GuessingError
 from subliminal.extensions import get_default_providers, get_default_refiners
 from subliminal.utils import get_parameters_from_signature, merge_extend_and_ignore_unions
 
@@ -654,39 +659,47 @@ def download(
             p = os.path.abspath(os.path.expanduser(p))
             logger.debug('Collecting path %s', p)
 
-            video_candidates: list[Video] = []
-
-            # non-existing
-            if not os.path.exists(p):
+            # collect files from directory
+            collected_filepaths = [p]
+            if os.path.isdir(p):
+                # collect video files
                 try:
-                    video = scan_name(p, name=name)
+                    collected_filepaths = collect_video_filepaths(p, age=age, archives=archives)
                 except ValueError:
-                    repl_p = f'{p} ({name})' if name else p
-                    logger.exception('Unexpected error while collecting non-existing path %s', repl_p)
+                    logger.exception('Unexpected error while collecting directory path %s', p)
                     errored_paths.append(p)
                     continue
-                video_candidates.append(video)
 
-            # directories
-            elif os.path.isdir(p):
+            # scan videos
+            video_candidates: list[Video] = []
+            for filepath in collected_filepaths:
+                exists = os.path.exists(p)
                 try:
-                    scanned_videos = scan_videos(p, age=age, archives=archives, name=name)
-                except ValueError:
-                    repl_p = f'{p} ({name})' if name else p
-                    logger.exception('Unexpected error while collecting directory path %s', repl_p)
-                    errored_paths.append(p)
+                    # existing path
+                    if exists:  # noqa: SIM108
+                        video = scan_video_or_archive(filepath, name=name)
+                    else:
+                        video = scan_name(filepath, name=name)
+
+                except GuessingError as e:
+                    # Show a simple error message
+                    if verbose > 0:
+                        # new line was already added with debug
+                        if not debug:
+                            click.echo()
+                        click.secho(e, fg='yellow')
+                    errored_paths.append(filepath)
                     continue
-                video_candidates.extend(scanned_videos)
 
-            # other inputs
-            else:
-                try:
-                    video = scan_video(p, name=name)
                 except ValueError:
-                    repl_p = f'{p} ({name})' if name else p
-                    logger.exception('Unexpected error while collecting path %s', repl_p)
-                    errored_paths.append(p)
+                    logger.exception(
+                        'Unexpected error while collecting %s %s',
+                        'path' if exists else 'non-existing path',
+                        f'{filepath} ({name})' if name else filepath,
+                    )
+                    errored_paths.append(filepath)
                     continue
+
                 video_candidates.append(video)
 
             # check and refine videos
@@ -740,11 +753,12 @@ def download(
     # exit if no providers are used
     if len(use_providers) == 0:
         click.echo('No provider was selected to download subtitles.')
-        if 'ALL' in ignore_provider:
-            click.echo('All ignored from CLI argument: `--ignore-provider=ALL`')
-        elif 'ALL' in obj['provider_lists']['ignore']:
-            config_ignore = list(obj['provider_lists']['ignore'])
-            click.echo(f'All ignored from configuration: `ignore_provider={config_ignore}`')
+        if verbose > 0:
+            if 'ALL' in ignore_provider:
+                click.echo('All ignored from CLI argument: `--ignore-provider=ALL`')
+            elif 'ALL' in obj['provider_lists']['ignore']:
+                config_ignore = list(obj['provider_lists']['ignore'])
+                click.echo(f'All ignored from configuration: `ignore_provider={config_ignore}`')
         return
 
     # download best subtitles
diff --git a/src/subliminal/core.py b/src/subliminal/core.py
index eb8c6e4..632c61f 100644
--- a/src/subliminal/core.py
+++ b/src/subliminal/core.py
@@ -515,26 +515,58 @@ def scan_video(path: str | os.PathLike, name: str | None = None) -> Video:
     return video
 
 
-def scan_videos(
+def scan_video_or_archive(path: str | os.PathLike, name: str | None = None) -> Video:
+    """Scan a video or an archive from a `path`.
+
+    :param str path: existing path to the video or archive.
+    :param str name: if defined, name to use with guessit instead of the path.
+    :return: the scanned video.
+    :rtype: :class:`~subliminal.video.Video`
+    :raises: :class:`ValueError`: video path is not well defined.
+    """
+    path = os.fspath(path)
+    # check for non-existing path
+    if not os.path.exists(path):
+        msg = 'Path does not exist'
+        raise ValueError(msg)
+
+    # scan
+    filename = os.path.basename(path)
+    if filename.lower().endswith(VIDEO_EXTENSIONS):
+        # scan video
+        return scan_video(path, name=name)
+
+    if is_supported_archive(filename):
+        # scan archive
+        try:
+            video = scan_archive(path, name=name)
+        except ArchiveError as e:
+            # Re-raise as a ValueError
+            msg = 'Error scanning archive'
+            raise ValueError(msg) from e
+        return video
+
+    msg = f'Unsupported file {filename!r}'  # pragma: no cover
+    raise ValueError(msg)  # pragma: no cover
+
+
+def collect_video_filepaths(
     path: str | os.PathLike,
     *,
     age: timedelta | None = None,
     use_ctime: bool = False,
     archives: bool = True,
     name: str | None = None,
-) -> list[Video]:
-    """Scan `path` for videos and their subtitles.
-
-    See :func:`refine` to find additional information for the video.
+) -> list[str]:
+    """Collect video file paths in directory `path`.
 
     :param str path: existing directory path to scan.
     :param datetime.timedelta age: maximum age of the video or archive.
     :param bool use_ctime: use the latest of creation time and modification time to compute the age of the video,
         instead of just modification time.
     :param bool archives: scan videos in archives.
-    :param str name: name to use with guessit instead of the path.
-    :return: the scanned videos.
-    :rtype: list of :class:`~subliminal.video.Video`
+    :return: the collected video file names.
+    :rtype: list of str
     :raises: :class:`ValueError`: video path is not well defined.
     """
     path = os.fspath(path)
@@ -549,7 +581,7 @@ def scan_videos(
         raise ValueError(msg)
 
     # walk the path
-    videos = []
+    video_filepaths = []
     for dirpath, dirnames, filenames in os.walk(path):
         logger.debug('Walking directory %r', dirpath)
 
@@ -599,24 +631,43 @@ def scan_videos(
                     logger.debug('Skipping old file %r in %r', filename, dirpath)
                     continue
 
-            # scan
-            if filename.lower().endswith(VIDEO_EXTENSIONS):  # video
-                try:
-                    video = scan_video(filepath, name=name)
-                except ValueError:  # pragma: no cover
-                    logger.exception('Error scanning video')
-                    continue
-            elif archives and is_supported_archive(filename):  # archive
-                try:
-                    video = scan_archive(filepath, name=name)
-                except (ArchiveError, ValueError):  # pragma: no cover
-                    logger.exception('Error scanning archive')
-                    continue
-            else:  # pragma: no cover
-                msg = f'Unsupported file {filename!r}'
-                raise ValueError(msg)
+            video_filepaths.append(filepath)
 
-            videos.append(video)
+    return video_filepaths
+
+
+def scan_videos(
+    path: str | os.PathLike,
+    *,
+    age: timedelta | None = None,
+    use_ctime: bool = False,
+    archives: bool = True,
+    name: str | None = None,
+) -> list[Video]:
+    """Scan `path` for videos and their subtitles.
+
+    See :func:`refine` to find additional information for the video.
+
+    :param str path: existing directory path to scan.
+    :param datetime.timedelta age: maximum age of the video or archive.
+    :param bool use_ctime: use the latest of creation time and modification time to compute the age of the video,
+        instead of just modification time.
+    :param bool archives: scan videos in archives.
+    :param str name: name to use with guessit instead of the path.
+    :return: the scanned videos.
+    :rtype: list of :class:`~subliminal.video.Video`
+    :raises: :class:`ValueError`: video path is not well defined.
+    """
+    filepaths = collect_video_filepaths(path, age=age, use_ctime=use_ctime, archives=archives)
+
+    videos = []
+    for filepath in filepaths:
+        try:
+            video = scan_video_or_archive(filepath, name=name)
+        except ValueError:
+            logger.exception('Error scanning video')
+            continue
+        videos.append(video)
 
     return videos
 
diff --git a/src/subliminal/exceptions.py b/src/subliminal/exceptions.py
index 4b3659b..85bab29 100644
--- a/src/subliminal/exceptions.py
+++ b/src/subliminal/exceptions.py
@@ -13,6 +13,12 @@ class ArchiveError(Error):
     pass
 
 
+class GuessingError(Error, ValueError):
+    """ValueError raised when guessit cannot guess a video."""
+
+    pass
+
+
 class ProviderError(Error):
     """Exception raised by providers."""
 
diff --git a/src/subliminal/video.py b/src/subliminal/video.py
index 084a559..8ee5ed0 100644
--- a/src/subliminal/video.py
+++ b/src/subliminal/video.py
@@ -9,6 +9,7 @@ from typing import TYPE_CHECKING, Any
 
 from guessit import guessit  # type: ignore[import-untyped]
 
+from subliminal.exceptions import GuessingError
 from subliminal.utils import ensure_list, get_age, matches_extended_title
 
 if TYPE_CHECKING:
@@ -258,7 +259,7 @@ class Video:
             return Movie.fromguess(name, guess)
 
         msg = 'The guess must be an episode or a movie guess'  # pragma: no-cover
-        raise ValueError(msg)
+        raise GuessingError(msg)
 
     @classmethod
     def fromname(cls, name: str) -> Video:
@@ -380,8 +381,8 @@ class Episode(Video):
             raise ValueError(msg)
 
         if 'title' not in guess or 'episode' not in guess:
-            msg = 'Insufficient data to process the guess'
-            raise ValueError(msg)
+            msg = f'Insufficient data to process the guess for {name!r}'
+            raise GuessingError(msg)
 
         return cls(
             name,
@@ -470,8 +471,8 @@ class Movie(Video):
             raise ValueError(msg)
 
         if 'title' not in guess:
-            msg = 'Insufficient data to process the guess'
-            raise ValueError(msg)
+            msg = f'Insufficient data to process the guess for {name!r}'
+            raise GuessingError(msg)
 
         return cls(
             name,


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 318af78..9d8ebdf 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -17,6 +17,7 @@ from subliminal.core import (
     scan_archive,
     scan_name,
     scan_video,
+    scan_video_or_archive,
     scan_videos,
     search_external_subtitles,
 )
@@ -193,9 +194,12 @@ def test_scan_video_episode(episodes: dict[str, Episode], tmp_path: Path, monkey
 
 
 def test_scan_video_path_does_not_exist(movies: dict[str, Movie]) -> None:
-    with pytest.raises(ValueError) as excinfo:
-        scan_video(movies['man_of_steel'].name)
-    assert str(excinfo.value) == 'Path does not exist'
+    path = movies['man_of_steel'].name
+    with pytest.raises(ValueError, match='Path does not exist'):
+        scan_video(path)
+
+    with pytest.raises(ValueError, match='Path does not exist'):
+        scan_video_or_archive(path)
 
 
 def test_scan_video_invalid_extension(
@@ -206,9 +210,8 @@ def test_scan_video_invalid_extension(
     monkeypatch.chdir(tmp_path)
     movie_name = os.path.splitext(movies['man_of_steel'].name)[0] + '.mp3'
     ensure(tmp_path / movie_name)
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(ValueError, match="'.mp3' is not a valid video extension"):
         scan_video(movie_name)
-    assert str(excinfo.value) == "'.mp3' is not a valid video extension"
 
 
 def test_scan_video_broken(mkv: dict[str, str], tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
@@ -299,6 +302,35 @@ def test_scan_video_movie_name_path_does_not_exist(movies: dict[str, Movie]) ->
     assert scanned_video.year == video.year
 
 
+def test_scan_archive_error(
+    rar: dict[str, str],
+) -> None:
+    if 'pwd-protected' not in rar:
+        return
+    path = rar['pwd-protected']
+    with pytest.raises(ArchiveError):
+        scan_archive(path)
+
+    with pytest.raises(ValueError, match='Error scanning archive'):
+        scan_video_or_archive(path)
+
+
+def test_scan_videos_error(
+    rar: dict[str, str],
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    if 'pwd-protected' not in rar:
+        return
+    folder = Path(rar['pwd-protected']).parent
+    # Test return without error
+    scan_videos(folder)
+
+    # But error was logged
+    for record in caplog.records:
+        assert record.levelname == 'ERROR'
+    assert 'Error scanning archive' in caplog.text
+
+
 def test_scan_archive_invalid_extension(
     movies: dict[str, Movie],
     tmp_path: Path,
diff --git a/tests/test_video.py b/tests/test_video.py
index 66858dd..c36c5e2 100644
--- a/tests/test_video.py
+++ b/tests/test_video.py
@@ -151,30 +151,26 @@ def test_episode_from_guess_multi_episode(episodes: dict[str, Episode]) -> None:
 
 def test_episode_fromguess_wrong_type(episodes: dict[str, Episode]) -> None:
     guess = {'type': 'subtitle'}
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(ValueError, match='The guess must be an episode guess'):
         Episode.fromguess(episodes['bbt_s07e05'].name, guess)
-    assert str(excinfo.value) == 'The guess must be an episode guess'
 
 
 def test_episode_fromguess_insufficient_data(episodes: dict[str, Episode]) -> None:
     guess = {'type': 'episode'}
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(ValueError, match='Insufficient data to process the guess'):
         Episode.fromguess(episodes['bbt_s07e05'].name, guess)
-    assert str(excinfo.value) == 'Insufficient data to process the guess'
 
 
 def test_movie_fromguess_wrong_type(movies: dict[str, Movie]) -> None:
     guess = {'type': 'subtitle'}
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(ValueError, match='The guess must be a movie guess'):
         Movie.fromguess(movies['man_of_steel'].name, guess)
-    assert str(excinfo.value) == 'The guess must be a movie guess'
 
 
 def test_movie_fromguess_insufficient_data(movies: dict[str, Movie]) -> None:
     guess = {'type': 'movie'}
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(ValueError, match='Insufficient data to process the guess'):
         Movie.fromguess(movies['man_of_steel'].name, guess)
-    assert str(excinfo.value) == 'Insufficient data to process the guess'
 
 
 def test_movie_fromname(movies: dict[str, Movie]) -> None: