Various changes

- Improved docstrings
- Return sorted lists
- Simplified `_getPerformers()`
- Other minor changes

Signed-off-by: JoeLametta <[email protected]>

whipper/common/mbngs.py

@@ -153,88 +153,78 @@ def getIds(self):
 
 
 def _getWorks(recording):
- """Get "performance of" works out of a recording."""
+ """
+ Get 'performance of' works out of a recording.
+
+ :param recording: recording entity in MusicBrainz
+ :type recording: list
+ :returns: list of works being a performance of a recording
+ :rtype: list
+ """
  works = []
- valid_work_rel_types = [
- 'a3005666-a872-32c3-ad06-98af558e99b0', # "Performance"
- ]
+ valid_type_id = 'a3005666-a872-32c3-ad06-98af558e99b0' # "Performance"
  if 'work-relation-list' in recording:
  for work in recording['work-relation-list']:
- if work['type-id'] in valid_work_rel_types:
+ if work['type-id'] == valid_type_id:
  works.append(work['work'])
  return works
 
 
 def _getWorksId(works):
- """Get work ids from "performance of" works"""
- ids = set() # using a set to prevent duplicates
+ """
+ Get work IDs from 'performance of' works.
+
+ :param works: list of works being a performance of a recording
+ :type works: list
+ :returns: sorted list of work IDs (without duplicates)
+ :rtype: list
+ """
+ ids = set()
  for work in works:
  ids.add(work['id'])
- # convert back to list since mutagen doesn't support set
- return list(ids)
+ return sorted(ids) # convert to list: mutagen doesn't support set
 
 
 def _getComposers(works):
  """
- Extract composer(s) from works' artist-relation-list into a list
+ Get composer(s) from works' artist-relation-list.
 
+ :param works: list of works being a performance of a recording
+ :type works: list
+ :returns: sorted list of composers (without duplicates)
  :rtype: list
  """
- composers = set() # using a set to prevent duplicates
- valid_artist_rel_type = [
- 'd59d99ea-23d4-4a80-b066-edca32ee158f', # "Composer"
- ]
+ composers = set()
+ valid_type_id = 'd59d99ea-23d4-4a80-b066-edca32ee158f' # "Composer"
  for work in works:
  if 'artist-relation-list' in work:
  for artist_relation in work['artist-relation-list']:
- if artist_relation['type-id'] in valid_artist_rel_type:
+ if artist_relation['type-id'] == valid_type_id:
  composerName = artist_relation['artist']['name']
  composers.add(composerName)
- # convert back to list since mutagen doesn't support set
- return list(composers)
+ return sorted(composers) # convert to list: mutagen doesn't support set
 
 
 def _getPerformers(recording):
  """
  Extract performer(s) from recordings' artist-relation-list
 
+ :param recording: recording entity in MusicBrainz
+ :type recording: list
+ :returns: sorted list of performers' names (without duplicates)
  :rtype: list
  """
- performers = []
- valid_type_id = [
+ performers = set()
+ valid_type_id = {
  '59054b12-01ac-43ee-a618-285fd397e461', # "Instruments"
  '0fdbe3c6-7700-4a31-ae54-b53f06ae1cfa', # "Vocals"
  '628a9658-f54c-4142-b0c0-95f031b544da' # "Performers"
- ]
- common_attribute_name = [
- 'guest', 'additional', 'solo'
- ]
+ }
  if 'artist-relation-list' in recording:
  for artist_relation in recording['artist-relation-list']:
  if artist_relation['type-id'] in valid_type_id:
- type = artist_relation['type']
- name = artist_relation['artist']['name']
-
- if 'attribute-list' in artist_relation:
- prefix = '' # additional guest solo
- others = [] # types of vocal, instruments
- for attribute in artist_relation['attribute-list']:
- if attribute in common_attribute_name:
- # this works since attributes' order is
- # alphabetical which will correctly match the given
- # standard order
- # see: https://musicbrainz.org/relationship/eb10f8a0-0f4c-4dce-aa47-87bcb2bc42f3 # noqa E501
- prefix += attribute + ' '
- else:
- others.append(attribute)
- # if "guest performer: artist", `suffix` is `type` by
- # default else "guest solo piano, organ : artist"
- suffix = type if len(others) == 0 else ', '.join(others)
- type = prefix + suffix
-
- performer = "%s: %s" % (type, name)
- performers.append(performer)
- return performers
+ performers.add(artist_relation['artist']['name'])
+ return sorted(performers) # convert to list: mutagen doesn't support set
 
 
 def _getMetadata(release, discid=None, country=None):

whipper/test/test_common_mbngs.py

@@ -46,8 +46,9 @@ def testTrackTitle(self):
 
  def testComposersAndPerformers(self):
  """
- Test if composers and performers are extracted properly
- see https:/whipper-team/whipper/issues/191
+ Test whether composers and performers are extracted properly.
+
+ See: https:/whipper-team/whipper/issues/191
  """
  # Using: Mama Said - Lenny Kravitz
  # https://musicbrainz.org/release/410f99f8-a876-3416-bd8e-42233a00a477
@@ -60,12 +61,8 @@ def testComposersAndPerformers(self):
  discid='bIOeHwHT0aZJiENIYjAmoNxCPuA-')
  track1 = metadata.tracks[0]
  self.assertEqual(track1.composers,
- list({'Hal Fredericks', 'Michael Kamen'}))
- self.assertEqual(track1.performers, ['bass guitar, guitar family, '
- 'mellotron, membranophone: Lenny '
- 'Kravitz',
- 'guest guitar: Slash',
- 'vocal: Lenny Kravitz'])
+ ['Hal Fredericks', 'Michael Kamen'])
+ self.assertEqual(track1.performers, ['Lenny Kravitz', 'Slash'])
 
  def test2MeterSessies10(self):
  # various artists, multiple artists per track