Repository navigation
Conversation
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
58675e5 to
054d241
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6893 +/- ##
=======================================
Coverage 77.44% 77.44%
=======================================
Files 163 163
Lines 21945 21957 +12
Branches 3386 3389 +3
=======================================
+ Hits 16995 17005 +10
- Misses 4128 4129 +1
- Partials 822 823 +1
🚀 New features to boost your workflow:
|
054d241 to
08557bd
Compare
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
251def4 to
e8e5265
Compare
e8e5265 to
95086dc
Compare
There was a problem hiding this comment.
Pull request overview
PR make lastgenre album stage smarter for multi-artist albums. When Last.fm give no album tags for main albumartist, plugin now try album lookup for each value in multi-valued albumartists, then still fall through to artist stage if nothing found.
Changes:
- Add
_try_resolve_album_stagehelper to hold album-stage logic and multi-valuedalbumartistsfallback. - Add focused unit test that exercises per-albumartist album lookup fallback.
- Document new multi-artist album behavior in
lastgenredocs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
beetsplug/lastgenre/__init__.py |
Add album-stage helper that falls back to per-albumartists album lookups when primary album lookup empty. |
test/plugins/test_lastgenre.py |
Add direct test for _try_resolve_album_stage fallback behavior. |
docs/plugins/lastgenre.rst |
Document album-stage fallback to multi-valued albumartists before artist source. |
95086dc to
e29ecb9
Compare
e29ecb9 to
2cd9998
Compare
## Description
The monolithic `_get_genre` method was broken down into several private
instance methods and refactored for readability. The contract is kept
and is already well tested (`test_get_genre`)
- **Core Helpers** - were moved from within `_get_genre` to a reusable
instance method and a `cached_property`:
- `_try_resolve_stage`: Handles the canonicalization and logging of
genres for a specific stage.
- `fallback`: Provides the configured fallback genre. Is used as a last
resort in `_try_resolve_existing_genres` and when `_get_genre` couldn't
find any genre in any stage at all.
- **Lookup Stages** - some were complex enough to deserve their own
instance method for readability, some stay inline in `_get_genre`:
- `_try_resolve_existing_genres`: Manages the initial check for
pre-existing genres and the `cleanup_existing` logic when `force` is
disabled.
- track stage: stays inline
- album stage: indentical to track stage, but not worth moving /
deduplication doesn't buy much (see subsequent PR though)
- `_fetch_artist_stage`: Fetches and resolves artist-level genres,
including multi-valued album artists and "Various Artists" logic.
- `_fetch_va_genres`: specifically handles the plurality logic for
"Various Artists" albums.
- **Fallbacks**:
- `_try_resolve_original_fallback`: Handles the "keep_existing" logic
that attempts to use/canonicalize originally present genres if no new
ones are found.
Make sure to also look at subsequent PR's:
- #6890
- #6893
## To Do
- [x] ~Documentation~
- [x] Changelog. (Not required, refactor only)
- [x] ~Tests~ (_get_genre was already well covered and the signature of
the method was kept)
587af49 to
aa451d8
Compare
Try and concatinate genres of each member of albumartists in the multi-valued field if no genre for the main albumartist could be found.
a53a8ae to
dcb1066
Compare
Description
Fixes: #6238
If last.fm returns no album genres for
albumartist, fall back to querying the album against each artist inalbumartists(falling through to the artist stage if that also yields nothing.)The album stage reasoning is now slightly more complex and was moved to its own helper method.
To Do