[FEATURE] Simplify `music_tags` plugin (#660)

* [FEATURE] Simplify ``music_tags`` plugin * docs * more compatible * check old format better * better dict instantiate
2023-07-24 18:05:54 -07:00 · 2023-07-24 18:05:54 -07:00 · cc38db9c7a
commit cc38db9c7a
parent 0cbd2af4d3
10 changed files with 127 additions and 42 deletions
--- a/docs/config.rst
+++ b/docs/config.rst
@ -194,8 +194,6 @@ match_filters
 music_tags
 ''''''''''
 .. autoclass:: ytdl_sub.plugins.music_tags.MusicTagsOptions()
-  :members:
-  :exclude-members: partial_validate, embed_thumbnail

 -------------------------------------------------------------------------------

--- a/docs/deprecation_notices.rst
+++ b/docs/deprecation_notices.rst
@ -4,17 +4,19 @@ Deprecation Notices
 July 2023
 ---------

-embed_thumbnail
-^^^^^^^^^^^^^^^
+music_tags
+^^^^^^^^^^

-Embedding thumbnails has its own dedicated plugin now, which supports both audio and video files.
-It will be removed from ``music_tags`` in October 2023. Convert from:
+Music tags is getting simplified. ``tags`` will now reside directly under music_tags, and
+``embed_thumbnail`` is getting moved to its own plugin (supports video files as well). Convert from:

 .. code-block:: yaml

   my_example_preset:
     music_tags:
       embed_thumbnail: True
+       tags:
+         artist: "Elvis Presley"

 To the following:

@ -22,4 +24,7 @@ To the following:

   my_example_preset:
     embed_thumbnail: True
+     music_tags:
+       artist: "Elvis Presley"

+The old format will be removed in October 2023.
--- a/examples/music_audio_config.yaml
+++ b/examples/music_audio_config.yaml
@ -64,17 +64,19 @@ presets:
    # It is recommended to keep most of this as-is, and use override
    # variables to set them to be what you want.
    music_tags:
-      tags:
-        artist: "{track_artist}"
-        artists: "{track_artist}"
-        albumartist: "{track_artist}"
-        albumartists: "{track_artist}"
-        title: "{track_title}"
-        album: "{track_album}"
-        track: "{track_number}"
-        tracktotal: "{track_total}"
-        year: "{track_year}"
-        genre: "{track_genre}"
+      artist: "{track_artist}"
+      artists: "{track_artist}"
+      albumartist: "{track_artist}"
+      albumartists: "{track_artist}"
+      title: "{track_title}"
+      album: "{track_album}"
+      track: "{track_number}"
+      tracktotal: "{track_total}"
+      year: "{track_year}"
+      genre: "{track_genre}"
+
+    # Optionally embed the thumbnail into the track
+    embed_thumbnail: False

    # For every configurable field, make it an override variable,
    # so we can carefully tune different use-cases by only modifying override variables.
--- a/src/ytdl_sub/plugins/music_tags.py
+++ b/src/ytdl_sub/plugins/music_tags.py
@ -1,3 +1,4 @@
+import copy
 from collections import defaultdict
 from typing import Any
 from typing import Dict
@ -65,32 +66,31 @@ class MusicTagsOptions(OptionsDictValidator):
             tags:
               artist: "{artist}"
               album: "{album}"
-               genre: "ytdl-sub"
               # Supports id3v2.4 multi-tags
+               genres:
+                 - "{genre}"
+                 - "ytdl-sub"
               albumartists:
                 - "{artist}"
                 - "ytdl-sub"
    """

-    _required_keys = {"tags"}
-    _optional_keys = {"embed_thumbnail"}
-
-    @classmethod
-    def partial_validate(cls, name: str, value: Any) -> None:
-        """
-        Partially validate music tags
-        """
-        if isinstance(value, dict):
-            value["tags"] = value.get("tags", {})
-        _ = cls(name, value)
+    _optional_keys = {"tags", "embed_thumbnail"}
+    _allow_extra_keys = True

    def __init__(self, name, value):
        super().__init__(name, value)

-        self._tags = self._validate_key(key="tags", validator=MusicTagsValidator)
        self._embed_thumbnail = self._validate_key_if_present(
-            key="embed_thumbnail", validator=BoolValidator, default=False
-        ).value
+            key="embed_thumbnail", validator=BoolValidator
+        )
+
+        new_tags_dict: Dict[str, Any] = copy.deepcopy(value)
+        old_tags_dict = new_tags_dict.pop("tags", {})
+        new_tags_dict.pop("embed_thumbnail", None)
+
+        self._is_old_format = len(old_tags_dict) > 0 or self._embed_thumbnail is not None
+        self._tags = MusicTagsValidator(name=name, value=dict(old_tags_dict, **new_tags_dict))

    @property
    def tags(self) -> MusicTagsValidator:
@ -105,7 +105,9 @@ class MusicTagsOptions(OptionsDictValidator):
        """
        Optional. Whether to embed the thumbnail into the audio file.
        """
-        return self._embed_thumbnail
+        if self._embed_thumbnail is None:
+            return False
+        return self._embed_thumbnail.value


 class MusicTagsPlugin(Plugin[MusicTagsOptions]):
@ -122,6 +124,18 @@ class MusicTagsPlugin(Plugin[MusicTagsOptions]):
                f"to audio using the audio_extract plugin."
            )

+        # pylint: disable=protected-access
+        if self.plugin_options._is_old_format:
+            logger.warning(
+                "music_tags.tags is now deprecated. Place your tags directly under music_tags "
+                "instead. The old format will be removed in October of 2023."
+            )
+        if self.plugin_options.embed_thumbnail:
+            logger.warning(
+                "music_tags.embed_thumbnail is also deprecated. Use the dedicated "
+                "embed_thumbnail plugin instead. This will be removed in October of 2023."
+            )
+
        # Resolve the tags into this dict
        tags_to_write: Dict[str, List[str]] = defaultdict(list)
        for tag_name, tag_formatters in self.plugin_options.tags.as_lists.items():
@ -147,10 +161,6 @@ class MusicTagsPlugin(Plugin[MusicTagsOptions]):
                    setattr(audio_file, tag_name, tag_value[0])

            if self.plugin_options.embed_thumbnail:
-                logger.warning(
-                    "music_tags.embed_thumbnail is now deprecated. Use the dedicated "
-                    "embed_thumbnail plugin instead. This will be removed in October of 2023."
-                )

                # convert the entry thumbnail so it is embedded as jpg
                convert_download_thumbnail(entry=entry)
--- a/tests/e2e/plugins/test_audio_extract.py
+++ b/tests/e2e/plugins/test_audio_extract.py
@ -6,11 +6,29 @@ from ytdl_sub.subscriptions.subscription import Subscription


@pytest.fixture
-def single_song_preset_dict(output_directory):
+def single_song_preset_dict_old_format(output_directory):
    return {
        "preset": "single",
        # test multi-tags
        "music_tags": {"embed_thumbnail": True, "tags": {"genres": ["multi_tag_1", "multi_tag_2"]}},
+        # download the worst format so it is fast
+        "ytdl_options": {
+            "format": "worst[ext=mp4]",
+            "postprocessor_args": {"ffmpeg": ["-bitexact"]},  # Must add this for reproducibility
+        },
+        "overrides": {
+            "url": "https://www.youtube.com/watch?v=2lAe1cqCOXo",
+            "music_directory": output_directory,
+        },
+    }
+
+
+@pytest.fixture
+def single_song_preset_dict(output_directory):
+    return {
+        "preset": "single",
+        # test multi-tags
+        "music_tags": {"genres": ["multi_tag_1", "multi_tag_2"]},
        # test the new embed_thumbnail plugin
        "embed_thumbnail": True,
        # download the worst format so it is fast
@ -44,7 +62,33 @@ def multiple_songs_preset_dict(output_directory):

 class TestAudioExtract:
    @pytest.mark.parametrize("dry_run", [True, False])
-    def test_audio_extract_single_song(
+    def test_audio_extract_single_song_old_format(
+        self,
+        music_audio_config,
+        single_song_preset_dict_old_format,
+        output_directory,
+        dry_run,
+    ):
+        subscription = Subscription.from_dict(
+            config=music_audio_config,
+            preset_name="single_song_test",
+            preset_dict=single_song_preset_dict_old_format,
+        )
+
+        transaction_log = subscription.download(dry_run=dry_run)
+        assert_transaction_log_matches(
+            output_directory=output_directory,
+            transaction_log=transaction_log,
+            transaction_log_summary_file_name="plugins/test_audio_extract_single_old_format.txt",
+        )
+        assert_expected_downloads(
+            output_directory=output_directory,
+            dry_run=dry_run,
+            expected_download_summary_file_name="plugins/test_audio_extract_single_old_format.json",
+        )
+
+    @pytest.mark.parametrize("dry_run", [True, False])
+    def test_audio_extract_single_song_new_format(
        self,
        music_audio_config,
        single_song_preset_dict,
--- a/tests/e2e/plugins/test_music_tags.py
+++ b/tests/e2e/plugins/test_music_tags.py
@ -15,7 +15,7 @@ def single_song_video_dict(output_directory):
        },
        "output_options": {"output_directory": output_directory, "file_name": "will_error.mp4"},
        # test multi-tags
-        "music_tags": {"embed_thumbnail": True, "tags": {"genres": ["multi_tag_1", "multi_tag_2"]}},
+        "music_tags": {"genres": ["multi_tag_1", "multi_tag_2"]},
        # download the worst format so it is fast
        "ytdl_options": {
            "format": "worst[ext=mp4]",
--- a/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_single.json
+++ b/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_single.json
@ -1,5 +1,5 @@
 {
  ".ytdl-sub-single_song_test-download-archive.json": "c8ff22ec3304c9f8dab18cedaed4e8b4",
-  "YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind/01 - YouTube Rewind 2019： For the Record ｜ #YouTubeRewind.mp3": "37b38834eda1293ad503e00dcff7c4dc",
+  "YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind/01 - YouTube Rewind 2019： For the Record ｜ #YouTubeRewind.mp3": "33c69b0dce605e9f78fe23246ce442dd",
  "YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind/folder.jpg": "50ee47c80f679029f5d3503bb91b045a"
 }
--- a/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_single_old_format.json
+++ b/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_single_old_format.json
@ -0,0 +1,5 @@
+{
+  ".ytdl-sub-single_song_test-download-archive.json": "c8ff22ec3304c9f8dab18cedaed4e8b4",
+  "YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind/01 - YouTube Rewind 2019： For the Record ｜ #YouTubeRewind.mp3": "37b38834eda1293ad503e00dcff7c4dc",
+  "YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind/folder.jpg": "50ee47c80f679029f5d3503bb91b045a"
+}
--- a/tests/resources/transaction_log_summaries/plugins/test_audio_extract_single.txt
+++ b/tests/resources/transaction_log_summaries/plugins/test_audio_extract_single.txt
@ -4,7 +4,7 @@ Files created:
  .ytdl-sub-single_song_test-download-archive.json
 {output_directory}/YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind
  01 - YouTube Rewind 2019： For the Record ｜ #YouTubeRewind.mp3
-    Embedded Thumbnail, Music Tags:
+    Music Tags:
      album: YouTube Rewind 2019: For the Record | #YouTubeRewind
      albumartist: YouTube
      albumartists: YouTube
--- a/tests/resources/transaction_log_summaries/plugins/test_audio_extract_single_old_format.txt
+++ b/tests/resources/transaction_log_summaries/plugins/test_audio_extract_single_old_format.txt
@ -0,0 +1,21 @@
+Files created:
+----------------------------------------
+{output_directory}
+  .ytdl-sub-single_song_test-download-archive.json
+{output_directory}/YouTube/[2019] YouTube Rewind 2019： For the Record ｜ #YouTubeRewind
+  01 - YouTube Rewind 2019： For the Record ｜ #YouTubeRewind.mp3
+    Embedded Thumbnail, Music Tags:
+      album: YouTube Rewind 2019: For the Record | #YouTubeRewind
+      albumartist: YouTube
+      albumartists: YouTube
+      artist: YouTube
+      artists: YouTube
+      genre: Unset
+      genres:
+        - multi_tag_1
+        - multi_tag_2
+      title: YouTube Rewind 2019: For the Record | #YouTubeRewind
+      track: 1
+      tracktotal: 1
+      year: 2019
+  folder.jpg