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

Added option 'Remove perfect albums (allowing multiple file extensions)' #224

Closed
wants to merge 1 commit into from

Conversation

dsparkplug
Copy link

@dsparkplug dsparkplug commented Jul 9, 2019

This option treats albums with multiple complete sets of track files but with different extensions as perfect.

See #222

…s)' which treats albums with mutiple complete sets of track files but with different extensions as perfect
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.

Please respect PEP8

I didn't test the PR yet, but I will soon.

def callback(self, objs):
for album in objs:

if (not(isinstance(album, Album))):
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to add parenthesis here:

if not isinstance(album, Album):


def callback(self, objs):
for album in objs:

Copy link
Collaborator

Choose a reason for hiding this comment

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

This whiteline should be removed. See PEP8 for Python style recommendations.

title = album.column('title')
log.info('Checking album perfection: %s', title)

if (album.loaded != True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Useless parenthesis and test can be done this way:

if album.loaded:

continue

unsavedFiles = album.get_num_unsaved_files()
if (unsavedFiles > 0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

if unsavedFiles:

continue

unmatchedFiles = album.get_num_unmatched_files()
if (unmatchedFiles > 0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

if unmatchedFiles:

#get dictionary of file extension counts
extDict = {}
for file in album.iterfiles():
_, ext = os.path.splitext(file.filename)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not use _ as placeholder, it is used for translations (_() is a function), just use unused

@zas zas requested a review from phw July 21, 2019 16:57
@zas
Copy link
Collaborator

zas commented Aug 24, 2019

@dsparkplug any update?

@Sophist-UK
Copy link
Contributor

IMO this is trying to do in a plugin what should be done in Picard proper i.e. because we need the icons to reflect the perfect nature of the album or otherwise - and this PR will essentially remove albums which are shown as needing saving in the UI. We need Picard to recognise perfect albums which have multiple formats.

@phw
Copy link
Member

phw commented Dec 26, 2021

As this hasn't gone anywhere I'm going to close this for now. Can be re-opened if needed.

And I agree with @Sophist-UK, we could at least have an option to treat a release with multiple files per track as completed. Maybe at least if each track has the same amount of files and they are of different formats.

@phw phw closed this Dec 26, 2021
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.

4 participants