Skip to content

[change] Update best_quality.py#3959

Merged
gazpachoking merged 2 commits intoFlexget:developfrom
diogosena:patch-2
Apr 2, 2024
Merged

[change] Update best_quality.py#3959
gazpachoking merged 2 commits intoFlexget:developfrom
diogosena:patch-2

Conversation

@diogosena
Copy link
Copy Markdown
Contributor

@diogosena diogosena commented Mar 23, 2024

Motivation for changes:

The best_quality plugin was just considering the first entry as the best quality, and that's not always true, many entries can have the same quality as the best.

Detailed changes:

Added comparison of every entry with the best quality.

@diogosena diogosena changed the title Update best_quality.py [fix]Update best_quality.py Mar 23, 2024
@diogosena diogosena changed the title [fix]Update best_quality.py [fix] Update best_quality.py Mar 23, 2024
@gazpachoking
Copy link
Copy Markdown
Member

Hmm, does this change the plugin from only ever acting on one entry to now acting on multiple ones? I can see how this might be desirable, but I'm also worried it's changing the behavior a bit much. Maybe I'm wrong though. Anyone else using this plugin that wants to weigh in?

@diogosena
Copy link
Copy Markdown
Contributor Author

diogosena commented Mar 23, 2024

Hmm, does this change the plugin from only ever acting on one entry to now acting on multiple ones? I can see how this might be desirable, but I'm also worried it's changing the behavior a bit much. Maybe I'm wrong though. Anyone else using this plugin that wants to weigh in?

Yes, "on best" can now act on every entry. I understand your concern, but the old behavior seems wrong and is very confusing, "on_best" could act just on one entry, and "on lower" could act on all the others, even if they all are the same quality as the best .
Besides, if someone needs only first entry, there's the unique plugin to use.

@diogosena
Copy link
Copy Markdown
Contributor Author

@gazpachoking maybe you prefer an option "single_best: [yes|no]", defaulting to yes, to have the same behavior as before?

@gazpachoking
Copy link
Copy Markdown
Member

Oh, yes, sorry for the delay. I meant to suggest something like that. I think one of the most common uses for this plugin would be to grab a single copy of something in the best quality available, so it makes sense to keep that as the default. Adding an option sounds fine though to have it operate in this new mode though.

The best_quality plugin was just considering the first entry as the best quality, and that's not always true, many entries can have the same quality as the best.

using "single_best = no" to activates this new mode
@diogosena diogosena changed the title [fix] Update best_quality.py [change] Update best_quality.py Apr 2, 2024
@diogosena
Copy link
Copy Markdown
Contributor Author

Oh, yes, sorry for the delay. I meant to suggest something like that. I think one of the most common uses for this plugin would be to grab a single copy of something in the best quality available, so it makes sense to keep that as the default. Adding an option sounds fine though to have it operate in this new mode though.

Just updated the pull request, and updated the wiki (thought that will need some kind of approval), feel free to revert if needed.

@gazpachoking
Copy link
Copy Markdown
Member

Sounds good, thanks!

@gazpachoking gazpachoking merged commit 3b74ed5 into Flexget:develop Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants