metabrainz / picard-plugins

Picard plugins: use 1.0 branch for Picard < 2.0 (python 2/Qt4) and 2.0 branch for Picard >= 2.0 (python 3/Qt5)
https://picard.musicbrainz.org/plugins/
145 stars 95 forks source link

[abbreviate_artistsort] exception when artist sorted name has no last name part #350

Open shutterfreak opened 1 year ago

shutterfreak commented 1 year ago

When an artist sorted name only consists of 1 word (e.g., "Josquin" (for Josquin des Prez"), the abbreviate_artistsort plugin throws an exception since surname is not defined in this case.

Here's an example:

Traceback (most recent call last):
  File "coverart\__init__.py", line 144, in next_in_queue
StopIteration

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "album.py", line 407, in _finalize_loading_track
  File "metadata.py", line 655, in run_track_metadata_processors
  File "plugin.py", line 257, in run
  File "C:\Users\Xxx\AppData\Local\MusicBrainz\Picard\plugins\abbreviate_artistsort.zip\abbreviate_artistsort.py", line 138, in abbreviate_artistsort
    while surname.split(None, 1)[0] == unsort.split(None, 1)[0]:
IndexError: list index out of range
Traceback (most recent call last):
  File "coverart\__init__.py", line 144, in next_in_queue
StopIteration

Consider checking for this edge case everywhere in the plugin.

Sophist-UK commented 1 year ago

Oops sorry. Unfortunately I don't have the time to fix this right now. Any volunteers?

Sophist-UK commented 1 year ago

I have taken a look at the code and I wrote this so long ago that despite the detailed comments I cannot immediately help with an indicator on how to fix this.

As an aside, upon reflection coming back to this so long after I wrote it (which gives some perspective) I do think that it might be nice to store an abbreviated version of the unsorted tag as well.

P.S. to @shutterfreak : Do you have "Standardize Artist Names" set in Options as suggested in the comments for this plugin? Does it help? (This is not an excuse for the exception which should not happen in any circumstances - just interested to know if it makes a difference.)

shutterfreak commented 1 year ago

P.S. to @shutterfreak : Do you have "Standardize Artist Names" set in Options as suggested in the comments for this plugin? Does it help? (This is not an excuse for the exception which should not happen in any circumstances - just interested to know if it makes a difference.)

Yes I do :-)

The problem occurs on any release featuring Josquin des Prez since apparently the sort name was "Josquin" whereas one would expect "des Prez, Josquin". I meanwhile filed an edit to the artist sort name for Josquin des Préz (and also for Pierre de la Rue).

shutterfreak commented 1 year ago

As an aside, upon reflection coming back to this so long after I wrote it (which gives some perspective) I do think that it might be nice to store an abbreviated version of the unsorted tag as well.

I believe you wrote the plugin after exchanging ideas with me about 10 years ago :-)

Today I made one small change where I replace "Bach, J. S." with an even more compact "Bach, JS" version by first replacing ". " with "" and then "." with "". Very convenient for optimizing file and folder names on the file system.

Sophist-UK commented 1 year ago

Well - given unlimited dev time, it might be nice to offer a UI choice of abbreviations methods:

I just don't have the time for any coding ATM, and I have two higher priority projects that need my time first when I get some.

sambhavnoobcoder commented 10 months ago

I'll gladly work on this @Sophist-UK if the issue is still open.

Sophist-UK commented 10 months ago

@sambhavnoobcoder - It's open source - so go for it!!

sambhavnoobcoder commented 10 months ago

hey @Sophist-UK , @shutterfreak how do I set up the developers and testing environment for this ? really want to contribute to this repo , but can't understand how to get a testing and development environment running . any tips or assistance in the same would be extremely helpful.

Sophist-UK commented 10 months ago

The simplest thing to do is to go to the plugins directory and unzip the plugin file into a directory - then delete the zip file.

You can then edit the plugin in an editor and when you start Picard the new code will run and you can test it.

The only pain is having to restart Picard every time to save a change to the code.

sambhavnoobcoder commented 10 months ago

actually so far , I have cloned the repo and was able to understand that all the code for all the plugins was inside the plugins directory , inside a directory made for the particular plugin . however I can seem to understand how to run Picard to test any changes I make in the plugin .could you help me with that ?

Sophist-UK commented 10 months ago

It's exactly as I said. Copy the folders and files into the plugins directory (removing the previously downloaded zip file) and start Picard and see whether it works as expected.

sambhavnoobcoder commented 10 months ago

hey @Sophist-UK , I opened a pr that fixes this issue . https://github.com/metabrainz/picard-plugins/pull/365 . kindly look into it and in case of any changes required , kindly inform me of the same . I'll be glad to help out in any way possible .

sambhavnoobcoder commented 10 months ago

hey @Sophist-UK , where can I find the music from Josquin des Prez for download whose data is in the MetaBrainz database for testing ? any music I am testing form the internet for this artist says no metadata found . Or perhaps you could suggest some alternate method or artist to test this method ? any suggestions would be appreciated .

Sophist-UK commented 10 months ago

You don't need music for testing - you can tell Picard to download metadata by putting the MBID into the search box.

sambhavnoobcoder commented 10 months ago

@Sophist-UK , I am struggling to recreate this issue , anytime I try for an album of Josquin des Prez , I only get a warning saying W: 22:27:57,235 //Users/sambhavdixit/Library/Preferences/MusicBrainz/Picard/plugins/abbreviate_artistsort.abbreviate_artistsort:161: Could not match surname 'des Prez' in remaining unsorted: Josquin; Capella Alamire , unlike the mentioned traceback . if you find the time , it would be great if you could perhaps identify any test cases breaking the plugin , as cant find any test cases that perform as mentioned in the issue .

Sophist-UK commented 10 months ago

I haven't used it for years and have no examples I can provide.

ShashankSrivastava2002 commented 9 months ago

hello @Sophist-UK sir, sir is this issue still open or unsolved cause i want to solve it.

Sophist-UK commented 9 months ago

@ShashankSrivastava2002 It is open source, so please go ahead.

ShashankSrivastava2002 commented 8 months ago

i have solved this to a extent and want to add the solution please help me to do so