beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.91k stars 1.82k forks source link

lastgenre: Add an option to append to existing genres #4750

Open arsaboo opened 1 year ago

arsaboo commented 1 year ago

The current implementation of lastgenre ignores any existing genres that a user may have tagged and completely overwrites the genres returned by lastgenre. It would be nice to have the option to append the new genres returned by LastGenre to those that are already available.

Proposed solution

Extend the plugin to add an append option.

sampsyo commented 1 year ago

Sounds like a reasonable addition!

JOJ0 commented 1 year ago

Maybe that is a start for fixing this? https://github.com/beetbox/beets/commit/03848c6bb948dbc3cbd9e9f82009c1df1366ed7f

arsaboo commented 1 year ago

Maybe that is a start for fixing this? 03848c6

Does it take care of the fact that the pre-existing genre may be duplicated, i.e., the pre-existing genre may also be included in the new genre tags?

JOJ0 commented 1 year ago

Nope doesnt yet. It might be a start you/we can work from. The main reason I started tinkering with lastgenre was that it always overwrites when more than one genre (comma separated) was existing in the genre field/tag because this check didnt handle it. It did work if just one single genre (not comma separated) was present in the tag.

I didnt look up existing issues though and wasnt aware you opened this 2 months ago already :-)

arsaboo commented 1 year ago

@JOJ0 I have also been working on something along those lines. I have a working prototype - https://github.com/arsaboo/beets/tree/append_genre

Ignore the all_genres part for now, which I only added so that I can see what genres are being obtained (so that I can clean them up later). It basically, reads the original genre tags (even if they are more than one) and append the new tags. Tags are deduplicated. Let me know what you think.

JOJ0 commented 1 year ago

Cant see much w the link. on mobile. travelling right now. maybe send commit link.

but probably we should combine our efforts at some point and things/behaviour could be made configurable.

my approach tries to only allow what genres.txt defines and also later (not in that commit) I added cleanup via aliases and regex replacements. For example genre "drum n bass" (and many other spellings) will be accepted either as an existing tag or when received from last.fm but will be "normalized" to "Drum And Bass" during all checks and also when finally being saved to library/tag.

the drawback currently is w that approach that unknown genres get lost, but exactly stuff like that should be configurable: keep unknown genres as in your approach or clean them up by normalizing. user should have the option.

JOJ0 commented 1 year ago

More details on what I tinkered so far: https://github.com/beetbox/beets/discussions/4765#discussioncomment-6060849

arsaboo commented 1 year ago

For example genre "drum n bass" (and many other spellings) will be accepted either as an existing tag or when received from last.fm but will be "normalized" to "Drum And Bass" during all checks and also when finally being saved to library/tag.

I don't think we need regex for this. We can use canonicalization to achieve the same. I agree, we should combine our efforts. Maybe I will create a draft PR for you to look at and try things out.

JOJ0 commented 1 year ago

Maybe we are misunderstanding each orher? I dont use cannonicalization to be honest. Maybe dnb was a bad example. Another one: "2 step garage" should be just "2-step"

whatlastgenre has 2 approaches to solve this: a simple alias table (fast), regex replacements (expensive)

Anyway, yeah, I'll try to submit the things I borrowed from whatlastgenre when Im back home around in 2 weeks.

Send me links to your commits or to a direct diff of your changes, your link doesnt work on mobile, I just see a lot of files of your fork, not your actual changes. I'm still curious :-) Thanks!

arsaboo commented 1 year ago

You should still be able to use canonicalization for that. All you need in your genre-trees is:

- 2 step
    - 2 step garage
    - 2-step garage

In fact, you can even have variants (e.g., 2-step garage) as indicated above. I have yet to come across an example that I cannot manage using canonicalization....try it out.

All this assumes we know what genres (and their variants) can be used in several tag sources. This is why I added the logging option that collects all the genre tags.

I have created a draft PR #https://github.com/beetbox/beets/pull/4811 for you to look at. It has most of the things that we discussed. Let me know what you think and let us get this done.

Another option (maybe in the long run) is to create a generic genre plugin that can take any genre source (something like wlg).

JOJ0 commented 1 year ago

Sure, but fixing spelling is not the purpose of canonicalization.

Thanks for the draft, I'll take a look when I'm back home from holiday.

arsaboo commented 1 year ago

Sure, but fixing spelling is not the purpose of canonicalization.

Why not? I like to think of canonicalization as just a way to organize my genres. In any case, it doesn't hurt to provide a regex option if users prefer to use that.