Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
arsaboo opened this issue Apr 10, 2023 · 12 comments · May be fixed by #4811
Open

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

arsaboo opened this issue Apr 10, 2023 · 12 comments · May be fixed by #4811
Labels
feature features we would like to implement

Comments

@arsaboo
Copy link
Contributor

arsaboo commented Apr 10, 2023

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 sampsyo added the feature features we would like to implement label Apr 10, 2023
@sampsyo
Copy link
Member

sampsyo commented Apr 10, 2023

Sounds like a reasonable addition!

@sampsyo sampsyo changed the title Add an option to append genres lastgenre: Add an option to append to existing genres Apr 10, 2023
@JOJ0
Copy link
Member

JOJ0 commented Jun 1, 2023

Maybe that is a start for fixing this? 03848c6

@arsaboo
Copy link
Contributor Author

arsaboo commented Jun 1, 2023

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
Copy link
Member

JOJ0 commented Jun 2, 2023

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
Copy link
Contributor Author

arsaboo commented Jun 2, 2023

@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
Copy link
Member

JOJ0 commented Jun 2, 2023

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
Copy link
Member

JOJ0 commented Jun 2, 2023

More details on what I tinkered so far: #4765 (comment)

@arsaboo
Copy link
Contributor Author

arsaboo commented Jun 2, 2023

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
Copy link
Member

JOJ0 commented Jun 3, 2023

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 arsaboo linked a pull request Jun 4, 2023 that will close this issue
3 tasks
@arsaboo
Copy link
Contributor Author

arsaboo commented Jun 4, 2023

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 ##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
Copy link
Member

JOJ0 commented Jun 4, 2023

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
Copy link
Contributor Author

arsaboo commented Jun 4, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants