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

PICARD-2935: apply genre filters and threshold before selecting minimal usage #2517

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

phw
Copy link
Member

@phw phw commented Jun 25, 2024

Summary

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences:

Problem

The genre filter options can give unexpected results, where less genres are used then the maximum number of genres configured, even though more genres are available that would match the criteria. There are two distinct issues:

  1. The genre list is reduced to the maximum allowed number of items before filters are applied.
  2. The max. count (used to filter out genres by minusage threshold) is taken over the entire list. It better should ignore filtered out genres.

Examples case 1:

You have the following tags with counts:

Pop: 5, Blues: 7, Rock: 5, Psychedelic: 4
You have a filter "-pop"

Number of genres is limited to max. 3.

Picard takes the first most used tags (Blues, Pop, Rock), then applies the filters.

Result: Blues, Rock

Better would be to ignore the filtered genre and use: Blues, Rock, Psychedelic

Example case 2

You have the following tags with counts:

Pop: 5, Blues: 11, Rock: 20, Psychedelic: 9
You have a filter "-rock"

The threshold is set to 50%.

Picard takes the first most used tags (Rock, Blues, Pop), then applies the filters and the threshold. For the threshold the most used genre (Rock=20) is used as the reference max. count, even if it gets removed.

Result: Blues

Better would be to ignore the filtered genre. Then the max. count would be Blues=11 and the result be: Blues, Psychedelic

Solution

Apply the filters first, then the minusage threshold on the filtered result. Take max. no. of genres from the resulting list.

Action

Additional actions required:

  • Update Picard documentation (please include a reference to this PR)
  • Other (please specify below)

Copy link
Collaborator

@zas zas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix! LGTM

@phw
Copy link
Member Author

phw commented Jun 25, 2024

It looses a bit the elegance of the generator implementation and I think we can't avoid iterating the list multiple times now. But this logic works much better :) I now need to retag some files.

@phw phw merged commit dbe1254 into metabrainz:master Jun 25, 2024
44 checks passed
@phw phw deleted the PICARD-2935-better-genre-filter branch June 25, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants