Skip to content

Commit

Permalink
Merge pull request #4714 from TypicalFence/gh-654
Browse files Browse the repository at this point in the history 
resolve transl-tracklisting relations for pseudo releases
  • Loading branch information
@sampsyo
sampsyo authored Jun 26, 2023
2 parents fb93d9e + 3f31876 commit 16a30f4
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 8 deletions.
89 changes: 82 additions & 7 deletions beets/autotag/mb.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ def get_message(self):
RELEASE_INCLUDES = ['artists', 'media', 'recordings', 'release-groups',
'labels', 'artist-credits', 'aliases',
'recording-level-rels', 'work-rels',
'work-level-rels', 'artist-rels', 'isrcs', 'url-rels']
'work-level-rels', 'artist-rels', 'isrcs',
'url-rels', 'release-rels']
BROWSE_INCLUDES = ['artist-credits', 'work-rels',
'artist-rels', 'recording-rels', 'release-rels']
if "work-level-rels" in musicbrainzngs.VALID_BROWSE_INCLUDES['recording']:
Expand Down Expand Up @@ -133,7 +134,7 @@ def _preferred_alias(aliases: List):
matches = []
for a in aliases:
if a['locale'] == locale and 'primary' in a and \
a.get('type', '').lower() not in ignored_alias_types:
a.get('type', '').lower() not in ignored_alias_types:
matches.append(a)

# Skip to the next locale if we have no matches
Expand Down Expand Up @@ -583,10 +584,10 @@ def album_info(release: Dict) -> beets.autotag.hooks.AlbumInfo:


def match_album(
artist: str,
album: str,
tracks: Optional[int] = None,
extra_tags: Optional[Dict[str, Any]] = None,
artist: str,
album: str,
tracks: Optional[int] = None,
extra_tags: Optional[Dict[str, Any]] = None,
) -> Iterator[beets.autotag.hooks.AlbumInfo]:
"""Searches for a single album ("release" in MusicBrainz parlance)
and returns an iterator over AlbumInfo objects. May raise a
Expand Down Expand Up @@ -670,6 +671,64 @@ def _parse_id(s: str) -> Optional[str]:
return None


def _is_translation(r):
_trans_key = 'transl-tracklisting'
return r['type'] == _trans_key and r['direction'] == "backward"


def _find_actual_release_from_pseudo_release(pseudo_rel: Dict) \
-> Optional[Dict]:
relations = pseudo_rel['release']["release-relation-list"]

# currently we only support trans(liter)ation's
translations = [r for r in relations if _is_translation(r)]

if not translations:
return None

actual_id = translations[0]['target']

return musicbrainzngs.get_release_by_id(actual_id,
RELEASE_INCLUDES)


def _merge_pseudo_and_actual_album(
pseudo: beets.autotag.hooks.AlbumInfo,
actual: beets.autotag.hooks.AlbumInfo
) -> Optional[beets.autotag.hooks.AlbumInfo]:
"""
Merges a pseudo release with its actual release.
This implementation is naive, it doesn't overwrite fields,
like status or ids.
According to the ticket PICARD-145, the main release id should be used.
But the ticket has been in limbo since over a decade now.
It also suggests the introduction of the tag `musicbrainz_pseudoreleaseid`,
but as of this field can't be found in any offical Picard docs,
hence why we did not implement that for now.
"""
merged = pseudo.copy()
from_actual = {k: actual[k] for k in [
"media",
"mediums",
"country",
"catalognum",
"year",
"month",
"day",
"original_year",
"original_month",
"original_day",
"label",
"asin",
"style",
"genre"
]}
merged.update(from_actual)
return merged


def album_for_id(releaseid: str) -> Optional[beets.autotag.hooks.AlbumInfo]:
"""Fetches an album by its MusicBrainz ID and returns an AlbumInfo
object or None if the album is not found. May raise a
Expand All @@ -683,13 +742,29 @@ def album_for_id(releaseid: str) -> Optional[beets.autotag.hooks.AlbumInfo]:
try:
res = musicbrainzngs.get_release_by_id(albumid,
RELEASE_INCLUDES)

# resolve linked release relations
actual_res = None

if res['release']['status'] == 'Pseudo-Release':

This comment has been minimized.

Sign in to view

Copy link
@jamesharding

jamesharding Jun 29, 2023 •
edited
Loading

Contributor

#4832 Looks like this is the offending line. @sampsyo @TypicalFence

This comment has been minimized.

Sign in to view

Copy link
@TypicalFence

TypicalFence Jul 1, 2023

Contributor

real sorry about that one >_<
actual_res = _find_actual_release_from_pseudo_release(res)

except musicbrainzngs.ResponseError:
log.debug('Album ID match failed.')
return None
except musicbrainzngs.MusicBrainzError as exc:
raise MusicBrainzAPIError(exc, 'get release by ID', albumid,
traceback.format_exc())
return album_info(res['release'])

# release is potentially a pseudo release
release = album_info(res['release'])

# should be None unless we're dealing with a pseudo release
if actual_res is not None:
actual_release = album_info(actual_res['release'])
return _merge_pseudo_and_actual_album(release, actual_release)
else:
return release


def track_for_id(releaseid: str) -> Optional[beets.autotag.hooks.TrackInfo]:
Expand Down
3 changes: 3 additions & 0 deletions beets/ui/commands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ def disambig_string(info):
disambig.append(info.catalognum)
if info.albumdisambig:
disambig.append(info.albumdisambig)
# Let the user differentiate between pseudo and actual releases.
if info.albumstatus == 'Pseudo-Release':
disambig.append(info.albumstatus)

if disambig:
return ', '.join(disambig)
Expand Down
2 changes: 2 additions & 0 deletions docs/changelog.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ for Python 3.6).

New features:

* resolve transl-tracklisting relations for pseudo releases and merge data with the actual release
:bug:`654`
* Fetchart: Use the right field (`spotify_album_id`) to obtain the Spotify album id
:bug:`4803`
* Prevent reimporting album if it is permanently removed from Spotify
Expand Down
3 changes: 2 additions & 1 deletion test/test_importer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1936,7 +1936,8 @@ def mocked_get_release_by_id(id_, includes=[], release_status=[],
}],
'release-group': {
'id': 'another-id',
}
},
'status': 'Official',
}
}

Expand Down
159 changes: 159 additions & 0 deletions test/test_mb.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,7 @@ def test_match_album(self):
'release': {
'title': 'hi',
'id': mbid,
'status': 'status',
'medium-list': [{
'track-list': [{
'id': 'baz',
Expand Down Expand Up @@ -648,6 +649,164 @@ def test_match_album_empty(self):
self.assertFalse(p.called)
self.assertEqual(ail, [])

def test_follow_pseudo_releases(self):
side_effect = [
{
'release': {
'title': 'pseudo',
'id': 'd2a6f856-b553-40a0-ac54-a321e8e2da02',
'status': 'Pseudo-Release',
'medium-list': [{
'track-list': [{
'id': 'baz',
'recording': {
'title': 'translated title',
'id': 'bar',
'length': 42,
},
'position': 9,
'number': 'A1',
}],
'position': 5,
}],
'artist-credit': [{
'artist': {
'name': 'some-artist',
'id': 'some-id',
},
}],
'release-group': {
'id': 'another-id',
},
'release-relation-list': [
{
'type': 'transl-tracklisting',
'target': 'd2a6f856-b553-40a0-ac54-a321e8e2da01',
'direction': 'backward'
}
]
}
},
{
'release': {
'title': 'actual',
'id': 'd2a6f856-b553-40a0-ac54-a321e8e2da01',
'status': 'Offical',
'medium-list': [{
'track-list': [{
'id': 'baz',
'recording': {
'title': 'original title',
'id': 'bar',
'length': 42,
},
'position': 9,
'number': 'A1',
}],
'position': 5,
}],
'artist-credit': [{
'artist': {
'name': 'some-artist',
'id': 'some-id',
},
}],
'release-group': {
'id': 'another-id',
},
'country': 'COUNTRY',
}
}
]

with mock.patch('musicbrainzngs.get_release_by_id') as gp:
gp.side_effect = side_effect
album = mb.album_for_id('d2a6f856-b553-40a0-ac54-a321e8e2da02')
self.assertEqual(album.country, 'COUNTRY')

def test_pseudo_releases_without_links(self):
side_effect = [{
'release': {
'title': 'pseudo',
'id': 'd2a6f856-b553-40a0-ac54-a321e8e2da02',
'status': 'Pseudo-Release',
'medium-list': [{
'track-list': [{
'id': 'baz',
'recording': {
'title': 'translated title',
'id': 'bar',
'length': 42,
},
'position': 9,
'number': 'A1',
}],
'position': 5,
}],
'artist-credit': [{
'artist': {
'name': 'some-artist',
'id': 'some-id',
},
}],
'release-group': {
'id': 'another-id',
},
'release-relation-list': []
}
},
]

with mock.patch('musicbrainzngs.get_release_by_id') as gp:
gp.side_effect = side_effect
album = mb.album_for_id('d2a6f856-b553-40a0-ac54-a321e8e2da02')
self.assertEqual(album.country, None)

def test_pseudo_releases_with_unsupported_links(self):
side_effect = [
{
'release': {
'title': 'pseudo',
'id': 'd2a6f856-b553-40a0-ac54-a321e8e2da02',
'status': 'Pseudo-Release',
'medium-list': [{
'track-list': [{
'id': 'baz',
'recording': {
'title': 'translated title',
'id': 'bar',
'length': 42,
},
'position': 9,
'number': 'A1',
}],
'position': 5,
}],
'artist-credit': [{
'artist': {
'name': 'some-artist',
'id': 'some-id',
},
}],
'release-group': {
'id': 'another-id',
},
'release-relation-list': [
{
'type': 'remaster',
'target': 'd2a6f856-b553-40a0-ac54-a321e8e2da01',
'direction': 'backward'
}
]
}
},
]

with mock.patch('musicbrainzngs.get_release_by_id') as gp:
gp.side_effect = side_effect
album = mb.album_for_id('d2a6f856-b553-40a0-ac54-a321e8e2da02')
self.assertEqual(album.country, None)


def suite():
return unittest.TestLoader().loadTestsFromName(__name__)
Expand Down

0 comments on commit 16a30f4

Please sign in to comment.