feat(lastgenre): Add fallback_original#6414
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6414 +/- ##
==========================================
+ Coverage 69.39% 69.41% +0.01%
==========================================
Files 141 141
Lines 18816 18820 +4
Branches 3068 3070 +2
==========================================
+ Hits 13057 13063 +6
+ Misses 5112 5111 -1
+ Partials 647 646 -1
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces the fallback_original configuration flag to the lastgenre plugin, addressing issue #6318. When enabled, it preserves existing genre tags rather than overwriting them with the configured fallback value (or deleting them) in cases where no valid genres can be found or none survive whitelist/canonicalization filtering.
Changes:
- Added
fallback_originalconfig option and integration logic inbeetsplug/lastgenre/__init__.pyfor both theforce=Trueandforce=False/cleanup_existing=Truecode paths. - Added two parametrized test cases to cover both the forced and non-forced
fallback_originalscenarios. - Added documentation for the new option in
docs/plugins/lastgenre.rstand a changelog entry indocs/changelog.rst.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
beetsplug/lastgenre/__init__.py |
Adds fallback_original default config and logic to preserve original genres when no valid genre can be resolved |
test/plugins/test_lastgenre.py |
Adds parametrized test cases for fallback_original under both force=True and force=False/cleanup_existing=True |
docs/plugins/lastgenre.rst |
Documents the new fallback_original option |
docs/changelog.rst |
Changelog entry for the new feature |
beetsplug/lastgenre/__init__.py
Outdated
| if genres and self.config["fallback_original"]: | ||
| return genres, "fallback + original, force" |
There was a problem hiding this comment.
The label "fallback + original, force" at line 508 is misleading. This code path is reachable regardless of whether force is set: when force=False and genres is initially empty (no pre-existing genres), and fallback_original=True, the code still reaches line 507 if all lookup stages fail to find any genres. In that scenario the condition evaluates to False (since genres is []), so it's harmless, but the label implies force is always involved which is not true by design. Consider a more neutral label like "fallback + original" that doesn't imply force mode.
There was a problem hiding this comment.
As copilot correctly deduced, in the case of no-force, but no genres, this path is never reached and must thereby result in a force = true. I will, however, include a force check to make this more clear.
f9dc92f to
df178c4
Compare
Adds the `fallback_original` flag. When no genres can be found or no genres match the canonical whitelist, setting this to `yes` will result in existing genres to be preserved, instead of deleting them or setting them to `fallback`. This is for people that would rather have a non-canonicalized genre info than fully loose the existing genre data.
df178c4 to
91dbea3
Compare
|
Nevermind. I thought about this a bit, and I'm still not 100% sure about my own usecase. |
Description
Fixes #6318
Adds the
fallback_originalflag.When no genres can be found or no genres match the canonical whitelist, setting this to
yeswill result in existing genres to be preserved, instead of deleting them or setting them tofallback.This is for people that would rather have a bit of non-canonicalized genre info than fully loose the existing genre data.
@JOJ0 This is the last feature pull request I have in the pipeline ;D
This is more of a QoL feature that is nice to have, but not super important.
In case you think this is too specific, feel free to close the MR and the related issue :)
Cheers and thanks for your work and the reviews!
To Do