feat(browse): add function to get a song's album ID #847
Conversation
|
Oh right, docs.... |
There was a problem hiding this comment.
Pull request overview
This PR adds a new function get_song_album_id to retrieve the album browse ID associated with a song. The implementation searches through menu items to find the one with an "ALBUM" icon type, rather than relying on a fixed index position, making it more robust.
- Added
get_song_album_idmethod to retrieve a song's album ID - Introduced navigation constants to support the new functionality
- Added comprehensive tests covering both positive and negative cases
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ytmusicapi/navigation.py | Added navigation constants for watch next responses and menu browsing paths |
| ytmusicapi/mixins/browsing.py | Implemented get_song_album_id method to retrieve album ID by searching menu items for ALBUM icon |
| tests/mixins/test_browsing.py | Added tests for the new function covering songs with and without albums |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| del response[k] | ||
| return response | ||
|
|
||
| def get_song_album_id(self, videoId: str) -> str | None: |
There was a problem hiding this comment.
Thanks for this, it looks quite useful. I noticed that the function only returns a small part of the data which is actually retrieved via the next endpoint.
To keep the number of functions in this library reasonable, the philosophy is generally to return as much data as possible (and useful) from the web response, see for example get_song.
Can you see other use cases for the returned data?
There was a problem hiding this comment.
The only other useful information I see is fetching the songs metadata, because there is a provided browseid in the original response that points to the song's metadata. That would require a second net request, through.
ytmusicapi/mixins/browsing.py
Outdated
| :return: Album browseId (starting with ``MPREb_``) or ``None`` if no album was found. | ||
| """ | ||
| response = self._send_request( | ||
| "next", {"videoId": videoId, "index": 0, "watchNextType": "WATCH_NEXT_TYPE_SKIP_VIDEO"} |
There was a problem hiding this comment.
The same endpoint is already implemented here:
ytmusicapi/ytmusicapi/mixins/watch.py
Line 140 in 0f906c0
The difference being the request parameters used.
Do you see potential for reducing code duplication?
There was a problem hiding this comment.
I don't see any overlap in the functionality of these functions, can you elaborate what you mean?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #847 +/- ##
=======================================
Coverage ? 95.64%
=======================================
Files ? 46
Lines ? 2547
Branches ? 0
=======================================
Hits ? 2436
Misses ? 111
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
2 participants
No description provided.