lastgenre: Improve original fallback (better use of aliases and count) - #6890
lastgenre: Improve original fallback (better use of aliases and count)#6890JOJ0 wants to merge 2 commits into
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. |
dfef785 to
a5f7d91
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## lastgenre_refactor_get_genre #6890 +/- ##
================================================================
- Coverage 75.67% 75.67% -0.01%
================================================================
Files 163 163
Lines 21418 21418
Branches 3385 3384 -1
================================================================
- Hits 16209 16208 -1
- Misses 4410 4411 +1
Partials 799 799
🚀 New features to boost your workflow:
|
c53cebb to
1906d93
Compare
1906d93 to
b9cfd57
Compare
The discrepancy: - Normalization might help to keep more whitelisted genres! - But running through try_resolve_stage would be less code - The difference being that the latter reduces existing genres to the configured count which is not really fitting with a "last resort fallback to original genres" The here suggested solution: - We don't run through try_resolve_stage instantly because we want to make sure the "count" setting doesnt kick out anything but we apply aliases before whitelist check.
b9cfd57 to
39927e7
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. |
There was a problem hiding this comment.
Pull request overview
PR make lastgenre original-genre fallback better when force + keep_existing + whitelist (+ canonical) is on. Goal is: do alias normalize + whitelist check without _resolve_genres path that can apply count too early, then only do canonical-parent fallback as last resort.
Changes:
- Change “original fallback” stage to alias-normalize originals, then run whitelist/ignore filtering directly (no
_try_resolve_stageyet). - Keep existing canonicalized-parent fallback via
_try_resolve_stage(...)when nothing survives whitelist after aliasing. - Update docstring to describe new fallback order.
| normalized = [ | ||
| norm if norm != g.lower() else g | ||
| for g in genres | ||
| if (norm := normalize_genre(self._log, self.alias_patterns, g)) | ||
| ] |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
beetsplug/lastgenre/init.py:517
- grug see new logic to avoid
countdropping good original genres before whitelist. grug want regression test so this not break later. add pytest case whereforce+keep_existing+whitelist+canonicalandcount=1, existing genres has non-whitelisted first then whitelisted later, and expect "original fallback" returns whitelisted one (not lost to early count).
# We do not run through try_resolve_stage yet because count could drop
# existing genres, but we still apply aliases before whitelist
# filtering.
normalized = [
norm if norm != g.lower() else g
for g in genres
if (norm := normalize_genre(self._log, self.alias_patterns, g))
]
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
When
_get_genreruns withforce,whitelistandcanonicalizationenabled, as a last resort it tries to fall back to the original genres. Of course this stage normalizes, canonicalizes, does all the stuff it's supposed to when resolving the stage. That also includes reducing genres to the configuredcount. Now in that case that could reduce the list of genres and is not exactly what a fall back to what we had before should look like.This PR changes the stage as follows:
We don't run through
_try_resolve_stageinstantly because we want to make sure thecountsetting doesn't kick out anything prematurely! We then applywhitelistchecks and return if something valid is found.The very last resort action is kept as we had it already: Use
keep_genres(the lowercased originals) to canonicalize and hope that now we find a whitelisted parent genre.Note: Requires #6474
To Do
Documentation.(Not required IMO)