- Rename method _dedup_genre, since it's only used for
finalizing/polishing existing genres.
- Return separator-delimited string already.
- Decide on not passing "separator" to methods, it's a config
setting available throughout the plugin. Assign to variable where
useful for readability though.
- In the force branch, remove re-assigning keep_genres to empty list.
- Fix a test. Existing genres are "polished" now, which means:
configured title_case is applied.
- Fix/add type hints on all touched and new methods
- If the keep_existing option is set, just remember everything for now.
- Dedup happening later on via _combine... _resolve_genres...
- Even knowing if whitelist or not is not important at this point.
Useless variables that only were introduced for temporary debug logging
while refactoring earlier. Get rid of them.
Co-authored-by: Šarūnas Nejus <snejus@protonmail.com>
The best place to log what we actually fetched from last.fm seems to be
here in _combine_and_label_genres. Leave out the existing genres we also
receive in this function - less is more.
- Return fetched genres as a list from _resolve_genres().
- Format, limit to count and join to delimited string in helper
function.
- Fix docstring.
- Leave a couple of temporary debug messages.
- Fix original genre fallback - just keep as-is.
When original genres were kept (keep_existing option), the final genre
count was "off". The reason was that reducing genres to that count is
handled in _resolve_genre which wasn't run.
- This fixes it by ensuring a run of _resolve_genre in
_combine_and_label_genres.
- There is a small caveat though: New genres have been run through
_resolve_genres already. When they are combined with the old ones,
they run through it again. Let's take this into account for now and
hope performance doesn't suffer too much.
- Refactor and simplify logic of _get_genre()
- Add a config validation function.
- New default force: yes, keep_existing: yes (closest to original
behaviour)
trying to get a little order in the chaos. Maybe reordering and/or
moving out of the main plugin logic would be a better idea for some
methods but don't put much more refactoring into this PR to keep it
readable.
- Handle genre combination logic in a well documented helper function
that also include type hints.
- Throughout the _get_genre function rename the result variable to
new_genres to make it clearly descriptive.
- Rewrite thze _get_genre function's docstring.
- Retrieving, filtering and deduplicating present genres of Items/Albums
via separate methods.
- Implement all four cases of behaviour as described in PR#4982
- Issues:
- There is quite some unnecessary spliting of genres from strings into
lists and the other way round happening throughout the plugin.
- In the case where existing genres get "augmented" with last.fm
genres, we might end up with _more_ genres than the configured
limit.
- Default to False.
- During PR#4982 discussions we came to the conclusion that the
following behaviour would be a good new default choice:
- Keep whitelisted existing genres
- Only Fetch last.fm genres for empty tags.
- To get this we also have to change the default of the force
option!!!
- Resulting in "force: no" and "keep_allowed: yes"; see Case 4 in
PR#4982 description
- Options are not put to use yet, just defined and defaults set!
When `lastgenre.source: track` is configured,
- `lastgenre -a` _should not_ fall back to the album level genre (by
making use of the with_album=False kwarg of the Libary's get method).
- `lastgenre -a`, when finally storing the genres of _an album_, should
_not_ also write the tracks genres (by making use of the inherit=False
kwarg of the Album's store method.
- Printing out album/item in default format could lead to unreadable
clutter depending on the user's configured formats.
- The album's name and the individual tracks' title should be just
sufficient to provide context as well readability.
- Log like this while importing as well as in standalone runs.
It was rather confusing that the lastgenre plugin, when handling
singletons, sometimes showed that it applied genres from last.fm and
sometimes didn't (it did only in debug log). This streamlines the
behaviour:
- Change debug to info log.
- Streamline wording.
- Display details about the track.
When `lastgenre.source: track` is configured,
- `lastgenre -a` _should not_ fall back to the album level genre (by
making use of the with_album=False kwarg of the Libary's get method).
- `lastgenre -a`, when finally storing the genres of _an album_, should
_not_ also write the tracks genres (by making use of the inherit=False
kwarg of the Album's store method.
In _resolv_genres wo do not have to explicitly check if self.c14n_branches is
not empty. The tree will have been loaded when self.canonicalize is truthy. Also
if self.c14n_branches would be empty the canonicalization is a no-op anyway...
This improves lastgenre's behaviour when the configuration option
`prefer_specific` is set but `canonical` is not.
Previously it would not set any tags. Now it does apply tags, sorted using the
canonicalization tree, but not canonicalized.
For this the default tree is loaded even when `canonical` is not set.
An extra check is added to only use it for canonicalization when `canonical` is
set.
