get_playlist: support audio playlists#696
get_playlist: support audio playlists#696sigma67 merged 1 commit intosigma67:mainfrom sgvictorino:get-audio-playlist
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #696 +/- ##
==========================================
+ Coverage 95.07% 95.11% +0.03%
==========================================
Files 40 40
Lines 2357 2376 +19
==========================================
+ Hits 2241 2260 +19
Misses 116 116
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
You're trying to support albums with Albums are meant to be retrieved with I don't see the value in supporting the same content with two different endpoints. |
|
I realize that right now it's maybe not convenient to call |
|
Please also following CONTRIBUTING.rst for more involved changes so wasted effort can be prevented: https://github.com/sigma67/ytmusicapi/blob/main/CONTRIBUTING.rst#pull-requests |
|
I think it's useful for listing the audio counterparts, while >>> yt = YTMusic()
>>> playlist = "OLAK5uy_kzi7k-3sR60kgaRt8HpK2zBN6jlNOMoHw"
>>> [(s["videoId"], s["videoType"]) for s in yt.get_album(yt.get_album_browse_id(playlist))["tracks"]]
[('S9bCLPwzSC0', 'MUSIC_VIDEO_TYPE_OMV'), ('6CQ8FIRzjnk', 'MUSIC_VIDEO_TYPE_ATV'), ('sbCe_L_hXi0', 'MUSIC_VIDEO_TYPE_ATV')]
>>> [(s["videoId"], s["videoType"]) for s in yt.get_playlist(playlist)["tracks"]]
[('clxG52zpxIg', 'MUSIC_VIDEO_TYPE_ATV'), ('6CQ8FIRzjnk', 'MUSIC_VIDEO_TYPE_ATV'), ('sbCe_L_hXi0', 'MUSIC_VIDEO_TYPE_ATV')] |
|
Ok fair, I guess they do actually return different content. I would accept this if you could move the separate parsing for audio playlists into a separate parsing function. Don't make the audioPlaylist codepath dependent on a property of the data, instead make it dependent on the playlistId. If it starts with OLA, use the separate function for parsing the data. This should avoid mangling the code with too many if else statements while keeping it somewhat readable. |
There was a problem hiding this comment.
Please move to parsers/playlists.py. Sorry if I wasn't clear here.
The idea is that the mixins contain only the public API and everything related to parsing goes into a parsers/xx file.
No description provided.