fix bug 3804 #3836
fix bug 3804 #3836
Conversation
Signed-off-by: Syco <alberto.rinaudo+zhds@gmail.com>
beetsplug/missing.py
Outdated
| print_(u"{} - {}".format(artist[0], release_title)) | ||
| for album in missing: | ||
| for album_info in hooks.albums_for_id(album['id']): | ||
| item = _album_item(album_info, album_info.album_id) |
There was a problem hiding this comment.
Sorry for being slow, but I don't quite understand what the deal is with the new _album_item filter. Can you explain a little more how this works?
There was a problem hiding this comment.
To use the format function I needed an object previously returned by _item, but _item expects a track_info object.
Because this is applied to albums I don't have a track_info object to pass to _item, hence I created a new filter called _album_item that accept an album_info.
beetsplug/missing.py
Outdated
| resp = musicbrainzngs.browse_release_groups(artist=artist[1]) | ||
| release_groups = resp['release-group-list'] | ||
| resp = musicbrainzngs.browse_releases(artist=artist[1]) | ||
| release_groups = resp['release-list'] |
There was a problem hiding this comment.
I'm wondering whether the changes from this PR should be hidden behind a command line flag (--show-releases, --detailed-releases?). Going from release groups to individual releases will substantially increase the amount of output (I'd imagine you'd easily see 5-10 times as many output lines for many artists), making it harder to parse visually. I'm not a user of this plugin myself, but I think I'd mostly be interested in knowing the release groups that I'm missing rather than each and every Vinyl, CD and digital release.
What do you think?
Style-wise, I think release_groups (and rg) below should be renamed to releases and r to avoid confusion.
There was a problem hiding this comment.
Sorry for the confusion with the first commit, I should have tested more before opening the PR.
This change does increase the output, but the original rg object contains only release group id, artist, and album name.
If we want allow a formatting string that uses any other placeholder it's necessary to get the full album object.
Signed-off-by: Syco <alberto.rinaudo+zhds@gmail.com>
Signed-off-by: Syco <alberto.rinaudo+zhds@gmail.com>
| for release_group in missing: | ||
| try: | ||
| releases_object = musicbrainzngs.browse_releases( | ||
| release_group=release_group['id'] | ||
| ) | ||
| albums_list = releases_object['release-list'] | ||
| except MusicBrainzError as err: | ||
| self._log.info( | ||
| u"Couldn't fetch info for release group '{}' - '{}'", | ||
| release_group['id'], err | ||
| ) | ||
| continue | ||
|
|
||
| for album in albums_list: | ||
| for album_info in hooks.albums_for_id(album['id']): | ||
| item = _album_item(album_info, album_info.album_id) | ||
| print_(format(item, fmt)) |
There was a problem hiding this comment.
So, to print_ format, I need an item created by the filter at the top of the page, but the original object in the original code was the output of musicbrainzngs.browse_release_groups, these objects didn't contain any of the required attributes.
So for every missing release group I get the list of missing albums per release group.
This way I now have all the variables and can use the format function using all the placeholder already defined in the documentation.
|
This generates too many requests to musicbrainz and is far too slow to be usable. |
Signed-off-by: Syco alberto.rinaudo+zhds@gmail.com
Description
Fixes #3804