-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add composer_sort as a tag #2529
Changes from 5 commits
c51ecd4
e3c3798
075e243
2a418a6
d4ff82e
4a17da8
23f172d
813b078
e444143
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,6 +144,7 @@ class TrackInfo(object): | |
- ``data_url``: The data source release URL. | ||
- ``lyricist``: individual track lyricist name | ||
- ``composer``: individual track composer name | ||
- ``composer_sort``: individual track composer sort name | ||
- ``arranger`: individual track arranger name | ||
- ``track_alt``: alternative track number (tape, vinyl, etc.) | ||
|
||
|
@@ -155,8 +156,8 @@ def __init__(self, title, track_id, artist=None, artist_id=None, | |
length=None, index=None, medium=None, medium_index=None, | ||
medium_total=None, artist_sort=None, disctitle=None, | ||
artist_credit=None, data_source=None, data_url=None, | ||
media=None, lyricist=None, composer=None, arranger=None, | ||
track_alt=None): | ||
media=None, lyricist=None, composer=None, composer_sort=None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a trailing space here. |
||
arranger=None, track_alt=None): | ||
self.title = title | ||
self.track_id = track_id | ||
self.artist = artist | ||
|
@@ -174,6 +175,7 @@ def __init__(self, title, track_id, artist=None, artist_id=None, | |
self.data_url = data_url | ||
self.lyricist = lyricist | ||
self.composer = composer | ||
self.composer_sort = composer_sort | ||
self.arranger = arranger | ||
self.track_alt = track_alt | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -207,6 +207,7 @@ def track_info(recording, index=None, medium=None, medium_index=None, | |
|
||
lyricist = [] | ||
composer = [] | ||
composer_sort = [] | ||
for work_relation in recording.get('work-relation-list', ()): | ||
if work_relation['type'] != 'performance': | ||
continue | ||
|
@@ -218,10 +219,12 @@ def track_info(recording, index=None, medium=None, medium_index=None, | |
lyricist.append(artist_relation['artist']['name']) | ||
elif type == 'composer': | ||
composer.append(artist_relation['artist']['name']) | ||
composer_sort.append(artist_relation['artist']['sort-name']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is now too long, which is why our linting test is failing. You'll need to wrap it to be under 80 characters, the easiest way being this: composer_sort.append(
artist_relation['artist']['sort-name']) |
||
if lyricist: | ||
info.lyricist = u', '.join(lyricist) | ||
if composer: | ||
info.composer = u', '.join(composer) | ||
info.composer_sort = u', '.join(composer_sort) | ||
|
||
arranger = [] | ||
for artist_relation in recording.get('artist-relation-list', ()): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1632,12 +1632,21 @@ def update(self, dict): | |
StorageStyle('LYRICIST'), | ||
ASFStorageStyle('WM/Writer'), | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow the style of the rest of the code and don't add a blank line here. |
||
composer = MediaField( | ||
MP3StorageStyle('TCOM'), | ||
MP4StorageStyle('\xa9wrt'), | ||
StorageStyle('COMPOSER'), | ||
ASFStorageStyle('WM/Composer'), | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or here. |
||
composer_sort = MediaField( | ||
MP3StorageStyle('TSOC'), | ||
MP4StorageStyle('soco'), | ||
StorageStyle('COMPOSERSORT'), | ||
ASFStorageStyle('WM/Composersortorder'), | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or here. |
||
arranger = MediaField( | ||
MP3PeopleStorageStyle('TIPL', involvement='arranger'), | ||
MP4StorageStyle('----:com.apple.iTunes:Arranger'), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,6 +68,7 @@ def item(lib=None): | |
genre=u'the genre', | ||
lyricist=u'the lyricist', | ||
composer=u'the composer', | ||
composer_sort=u'the sortname of the composer', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no |
||
arranger=u'the arranger', | ||
grouping=u'the grouping', | ||
year=1, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,6 +141,12 @@ def test_add_image_structure(self): | |
type=ImageType.composer) | ||
mediafile.images += [image] | ||
mediafile.save() | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't have whitespace on (otherwise) empty lines. |
||
image = Image(data=self.png_data, desc=u'the sortname of the composer', | ||
type=ImageType.composer_sort) | ||
mediafile.images += [image] | ||
mediafile.save() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAICT, this is a test relating to images. I'm not sure |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for double blank lines here. |
||
|
||
mediafile = MediaFile(mediafile.path) | ||
self.assertEqual(len(mediafile.images), 3) | ||
|
@@ -150,6 +156,12 @@ def test_add_image_structure(self): | |
self.assertExtendedImageAttributes( | ||
image, desc=u'the composer', type=ImageType.composer | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't include whitespace on blank lines. |
||
images = (i for i in mediafile.images if i.desc == u'the sortname of the composer') | ||
image = next(images, None) | ||
self.assertExtendedImageAttributes( | ||
image, desc=u'the sortname of the composer', type=ImageType.composer_sort | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, not sure this test is relevant here. |
||
|
||
def test_delete_image_structures(self): | ||
mediafile = self._mediafile_fixture('image') | ||
|
@@ -193,6 +205,11 @@ def test_add_tiff_image(self): | |
type=ImageType.composer) | ||
mediafile.images += [image] | ||
mediafile.save() | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whitespace, blank line… |
||
image = Image(data=self.tiff_data, desc=u'the sortname of the composer', | ||
type=ImageType.composer_sort) | ||
mediafile.images += [image] | ||
mediafile.save() | ||
|
||
mediafile = MediaFile(mediafile.path) | ||
self.assertEqual(len(mediafile.images), 3) | ||
|
@@ -202,6 +219,11 @@ def test_add_tiff_image(self): | |
mediafile.images))[0] | ||
self.assertExtendedImageAttributes( | ||
image, desc=u'the composer', type=ImageType.composer) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whitespace, blank line… |
||
image = list(filter(lambda i: i.mime_type == 'image/tiff', | ||
mediafile.images))[0] | ||
self.assertExtendedImageAttributes( | ||
image, desc=u'the sortname of the composer', type=ImageType.composer_sort) | ||
|
||
|
||
class LazySaveTestMixin(object): | ||
|
@@ -318,29 +340,30 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, | |
""" | ||
|
||
full_initial_tags = { | ||
'title': u'full', | ||
'artist': u'the artist', | ||
'album': u'the album', | ||
'genre': u'the genre', | ||
'composer': u'the composer', | ||
'grouping': u'the grouping', | ||
'year': 2001, | ||
'month': None, | ||
'day': None, | ||
'date': datetime.date(2001, 1, 1), | ||
'track': 2, | ||
'tracktotal': 3, | ||
'disc': 4, | ||
'disctotal': 5, | ||
'lyrics': u'the lyrics', | ||
'comments': u'the comments', | ||
'bpm': 6, | ||
'comp': True, | ||
'mb_trackid': '8b882575-08a5-4452-a7a7-cbb8a1531f9e', | ||
'mb_albumid': '9e873859-8aa4-4790-b985-5a953e8ef628', | ||
'mb_artistid': '7cf0ea9d-86b9-4dad-ba9e-2355a64899ea', | ||
'art': None, | ||
'label': u'the label', | ||
'title': u'full', | ||
'artist': u'the artist', | ||
'album': u'the album', | ||
'genre': u'the genre', | ||
'composer': u'the composer', | ||
'composer_sort': u'the sortname of the composer', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. true, but I wonder why there isn't artist_sort . Imo where there is a name there also should be the sort name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. anyway, removed it. |
||
'grouping': u'the grouping', | ||
'year': 2001, | ||
'month': None, | ||
'day': None, | ||
'date': datetime.date(2001, 1, 1), | ||
'track': 2, | ||
'tracktotal': 3, | ||
'disc': 4, | ||
'disctotal': 5, | ||
'lyrics': u'the lyrics', | ||
'comments': u'the comments', | ||
'bpm': 6, | ||
'comp': True, | ||
'mb_trackid': '8b882575-08a5-4452-a7a7-cbb8a1531f9e', | ||
'mb_albumid': '9e873859-8aa4-4790-b985-5a953e8ef628', | ||
'mb_artistid': '7cf0ea9d-86b9-4dad-ba9e-2355a64899ea', | ||
'art': None, | ||
'label': u'the label', | ||
} | ||
|
||
tag_fields = [ | ||
|
@@ -350,6 +373,7 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, | |
'genre', | ||
'lyricist', | ||
'composer', | ||
'composer_sort', | ||
'arranger', | ||
'grouping', | ||
'year', | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remove these blank lines.