Merge pull request #6 from sheriferson/chore/generalize-classes-prep

Prepare the way for other lookups besides MusicBrainz + a gum choose tweak.

src/scrobble/app.py

@@ -6,13 +6,12 @@
 from typing_extensions import Annotated
 
 from scrobble.lastfm import get_lastfm_client
-from scrobble.musicbrainz import CD, UserAgent, init_musicbrainz
+from scrobble.musicbrainz import MusicBrainzCD, UserAgent, init_musicbrainz
 from scrobble.pushover import send_notification
 from scrobble.utils import prepare_tracks, choose_tracks
 
 import importlib.metadata
 
-
 USERAGENT = UserAgent('scrobble (PyPI)',
                       importlib.metadata.version('scrobble'),
                       'https://github.com/sheriferson/scrobble'
@@ -73,13 +72,14 @@ def cd(
         else:
             raise ValueError(f"The barcode you entered: {barcode} is not a number or a valid path to a barcode image.")
 
-    scrobble_cd: CD = CD.find_cd(resolved_barcode, release_choice)
+    scrobble_cd: MusicBrainzCD = MusicBrainzCD.find_cd(resolved_barcode, release_choice)
 
     if track_choice:
         tracks_to_scrobble = choose_tracks(scrobble_cd.tracks)
     else:
         tracks_to_scrobble = scrobble_cd.tracks
 
+
     prepped_tracks = prepare_tracks(scrobble_cd, tracks_to_scrobble, playback_end)
 
     if verbose:

src/scrobble/models/cd.py

@@ -0,0 +1,12 @@
+from abc import ABC
+
+from typing import Optional
+from scrobble.models.track import Track
+
+class CD(ABC):
+    id: str
+    title: str
+    artist: str
+    year: Optional[str]
+    discs: int
+    tracks: Optional[list[Track]] = None

src/scrobble/models/track.py

@@ -0,0 +1,21 @@
+from dataclasses import dataclass
+from typing import Optional
+from abc import ABC, abstractmethod
+
+@dataclass
+class Track(ABC):
+    track_title: str
+    track_artist: Optional[str]
+    disc_no: Optional[int]
+    track_position: int
+    track_length: int
+
+    def __str__(self):
+        pass
+
+    @property
+    @abstractmethod
+    def artist(self):
+        pass
+    def parse_source_result(cls, result: dict, dics_no: Optional[int] = 1):
+        pass

src/scrobble/musicbrainz.py

@@ -1,11 +1,21 @@
 from dataclasses import dataclass
-from typing import Optional
+from typing import Optional, Union
 
 import musicbrainzngs
 from dateutil import parser
 from rich import print
 from rich.prompt import IntPrompt
 
+from scrobble.models.track import Track
+from scrobble.models.cd import CD
+
+DISC_NO_DECORATION = {
+    '1': '₁',
+    '2': '₂',
+    '3': '₃',
+    '4': '₄'
+}
+
 
 @dataclass
 class UserAgent:
@@ -19,41 +29,56 @@ class UserAgent:
 
 
 @dataclass
-class Track:
-    track_title: str
-    disc_no: Optional[int]
-    track_position: int
-    track_length: int
+class MusicBrainzTrack(Track):
 
+    @property
+    def artist(self):
+        if self.track_artist:
+            return self.track_artist
+        else:
+            return ''
     @classmethod
-    def parse_musicbrainz_result(cls, result: dict, disc_no: Optional[int] = 1):
+    def parse_source_result(cls, result: dict, disc_no: Optional[int] = 1):
         """
         Look deep into the eyes of the json response and extract the track values we care about.
         """
         track_position: int = int(result['position'])
         title: str = result['recording']['title']
+        track_artist: str = result['recording']['artist-credit'][0]['artist']['name']
         if 'length' in result:
-            length: int = int(result['length']) / 1000
+            length: Union[int, float] = int(result['length']) / 1000
         elif 'track_or_recording_length' in result:
-            length: int = int(result['track_or_recording_length']) / 1000
+            length: Union[int, float] = int(result['track_or_recording_length']) / 1000
+        else:
+            raise RuntimeError(f"Couldn't find the length of track {result}")
 
-        return Track(title, disc_no, track_position, length)
+        return MusicBrainzTrack(title, track_artist, disc_no, track_position, length)
 
     def __str__(self):
-        return f"🎵 {self.track_position} {self.track_title}"
+        if self.disc_no:
+            return "💿{:>1} {:>2} {} - {}".format(DISC_NO_DECORATION[str(self.disc_no)], self.track_position, self.artist, self.track_title)
+        else:
+            return "🎵{:>2} {} - {}".format(self.track_position, self.artist, self.track_title)
 
 
 @dataclass
-class CD:
+class MusicBrainzCD(CD):
     id: str
     title: str
     artist: str
     year: Optional[str]
     discs: int
-    tracks: Optional[list[Track]] = None
+    _tracks: Optional[list[MusicBrainzTrack]] = None
+
+    @property
+    def tracks(self):
+        if not self._tracks:
+            self._tracks = self._get_tracks()
+        return self._tracks
 
-    def __post_init__(self):
-        self._get_tracks()
+    @tracks.setter
+    def tracks(self, new_tracks: Optional[list[MusicBrainzTrack]]):
+        self._tracks = new_tracks
 
     @classmethod
     def find_cd(cls, barcode: str, choice: bool = True):
@@ -67,7 +92,7 @@ def find_cd(cls, barcode: str, choice: bool = True):
             raise RuntimeError(f"No releases found for {barcode}")
         else:
             releases = results['release-list']
-            cds: list[CD] = [CD._parse_musicbrainz_result(release) for release in releases]
+            cds: list[MusicBrainzCD] = [MusicBrainzCD._parse_musicbrainz_result(release) for release in releases]
 
             if len(cds) < 2 or (not choice):
                 return cds[0]
@@ -98,19 +123,19 @@ def _parse_musicbrainz_result(cls, result: dict):
         year: str = str(parser.parse(result.get('date')).year) if 'date' in result and result['date'] else None
         disc_count: int = len(result['medium-list'])
 
-        return CD(id, title, artist, year, disc_count)
+        return MusicBrainzCD(id, title, artist, year, disc_count)
 
-    def _get_tracks(self) -> list[Track]:
+    def _get_tracks(self) -> list[MusicBrainzTrack]:
         """
         Call MusicBrainz to get the track list for all CDs in the release.
         """
-        result = musicbrainzngs.get_release_by_id(self.id, includes=['recordings'])
-        self.tracks: list[Track] = []
+        result = musicbrainzngs.get_release_by_id(self.id, includes=['recordings', 'artist-credits'])
+        retrieved_tracks: list[MusicBrainzTrack] = []
         for disc in result['release']['medium-list']:
-            self.tracks.extend([Track.parse_musicbrainz_result(track_result, disc['position'])
+            retrieved_tracks.extend([MusicBrainzTrack.parse_source_result(track_result, disc['position'])
                                 for track_result in disc['track-list']])
 
-        return self.tracks
+        return retrieved_tracks
 
     def __str__(self):
         return f"💿 {self.artist} - {self.title} ({self.year})"

src/scrobble/pushover.py

@@ -1,11 +1,11 @@
 import http.client
 import urllib
 
-from scrobble.musicbrainz import CD
+from scrobble.musicbrainz import MusicBrainzCD
 from scrobble.utils import Config
 
 
-def send_notification(cd: CD, config=None):
+def send_notification(cd: MusicBrainzCD, config=None):
     if not config:
         config = Config()
     conn = http.client.HTTPSConnection("api.pushover.net:443")

src/scrobble/utils.py

@@ -5,7 +5,7 @@
 import subprocess
 from typing import Optional
 
-from scrobble.musicbrainz import CD
+from scrobble.musicbrainz import MusicBrainzCD
 from scrobble.musicbrainz import Track
 
 
@@ -87,7 +87,7 @@ def pushover_user(self, new_pushover_user: str):
             raise ValueError('You cannot set the Pushover user key to an empty value.')
 
 
-def prepare_tracks(cd: CD, tracks: list[Track], playback_end: str = 'now') -> list[dict]:
+def prepare_tracks(cd: MusicBrainzCD, tracks: list[Track], playback_end: str = 'now') -> list[dict]:
     total_run_time: int = 0
     for track in tracks:
         total_run_time += track.track_length
@@ -112,7 +112,7 @@ def prepare_tracks(cd: CD, tracks: list[Track], playback_end: str = 'now') -> li
         elapsed += track.track_length
         prepped_tracks.append(
             {
-                'artist': cd.artist,
+                'artist': track.artist,
                 'title': track.track_title,
                 'album': cd.title,
                 'timestamp': start_time + elapsed
@@ -142,8 +142,8 @@ def choose_tracks(tracks: list[Track]) -> list[Track]:
         choices = ' '.join(['"' + track_str + '"' for track_str in track_dict.keys()])
         pre_selections = ','.join(['"' + track_str + '"' for track_str in track_dict.keys()])
         picked_tracks = subprocess.check_output(
-            f"{gum_path} choose {choices} --no-limit --selected={pre_selections}",
-            env={'GUM_CHOOSE_HEIGHT': str(len(tracks))},
+            f"{gum_path} choose --no-limit --selected={pre_selections} {choices}",
+            env={'GUM_CHOOSE_HEIGHT': str(len(tracks) + 2)},  # the +2 is weird, without it the list scrolls
             shell=True,
             encoding='UTF-8').rstrip()
 

tests/test__cd.py

@@ -1,6 +1,6 @@
 import pytest
 
-from scrobble.musicbrainz import CD, UserAgent, init_musicbrainz
+from scrobble.musicbrainz import MusicBrainzCD, UserAgent, init_musicbrainz
 
 import importlib.metadata
 
@@ -10,7 +10,7 @@
                       )
 
 init_musicbrainz(USERAGENT)
-TEST_CD = CD.find_cd(7277017746006, choice=False)
+TEST_CD = MusicBrainzCD.find_cd(7277017746006, choice=False)
 
 
 def test_cd_artist():
@@ -26,15 +26,17 @@ def test_cd_track_length():
 
 
 def test_cd_track_length_alt_attribute_name():
-    alt_test_cd: CD = CD.find_cd(4988005346872, choice=False)
+    alt_test_cd: MusicBrainzCD = MusicBrainzCD.find_cd(4988005346872, choice=False)
     assert len(alt_test_cd) == 15
     assert alt_test_cd.tracks[0].track_length > 0
 
+def test_cd_track_artist():
+    assert TEST_CD.tracks[0].artist is not None
 
 def test_cd_string_representation():
     assert str(TEST_CD) == "💿 Lacuna Coil - Comalies (2002)"
 
 
 def test_failed_CD_retrieval():
     with pytest.raises(RuntimeError):
-        CD.find_cd(12345)
+        MusicBrainzCD.find_cd(12345)

tests/test__pushover.py

@@ -1,6 +1,6 @@
 from scrobble.pushover import send_notification
 from scrobble.utils import Config
-from scrobble.musicbrainz import CD, UserAgent, init_musicbrainz
+from scrobble.musicbrainz import MusicBrainzCD, UserAgent, init_musicbrainz
 import importlib.metadata
 from urllib.parse import quote_plus
 
@@ -14,7 +14,7 @@
 
 init_musicbrainz(USERAGENT)
 
-TEST_CD = CD.find_cd(7277017746006, choice=False)
+TEST_CD = MusicBrainzCD.find_cd(7277017746006, choice=False)
 
 
 class TestSendNotification(unittest.TestCase):

tests/test__utils.py

@@ -2,14 +2,14 @@
 from unittest.mock import patch
 
 from scrobble.utils import Config, find_command, choose_tracks
-from scrobble.musicbrainz import Track
+from scrobble.musicbrainz import MusicBrainzTrack
 from typing import List
 
 
-TEST_TRACKS: List[Track] = [
-            Track(track_title="track1", disc_no=1, track_position=1, track_length=10),
-            Track(track_title="track1", disc_no=1, track_position=2, track_length=20),
-            Track(track_title="track1", disc_no=1, track_position=3, track_length=30),
+TEST_TRACKS: List[MusicBrainzTrack] = [
+            MusicBrainzTrack(track_title="track1", track_artist="The Artist", disc_no=1, track_position=1, track_length=10),
+            MusicBrainzTrack(track_title="track1", track_artist="The Artist", disc_no=1, track_position=2, track_length=20),
+            MusicBrainzTrack(track_title="track1", track_artist="The Artist", disc_no=1, track_position=3, track_length=30),
         ]
 
 def test_valid_config_has_lastfm_api():
@@ -53,9 +53,8 @@ def test_choose_tracks_with_gum(self, mock_check_output, mock_find_command):
 
         # Assert that the subprocess.check_output was called with the correct arguments
         mock_check_output.assert_called_once_with(
-            f'/path/to/gum choose "{str(TEST_TRACKS[0])}" "{str(TEST_TRACKS[1])}" "{str(TEST_TRACKS[2])}"'
-            f' --no-limit --selected="{str(TEST_TRACKS[0])}","{str(TEST_TRACKS[1])}","{str(TEST_TRACKS[2])}"',
-            env={'GUM_CHOOSE_HEIGHT': '3'},
+            f'/path/to/gum choose --no-limit --selected="{str(TEST_TRACKS[0])}","{str(TEST_TRACKS[1])}","{str(TEST_TRACKS[2])}" "{str(TEST_TRACKS[0])}" "{str(TEST_TRACKS[1])}" "{str(TEST_TRACKS[2])}"',
+            env={'GUM_CHOOSE_HEIGHT': '5'},
             shell=True,
             encoding='UTF-8'
         )