new generic TrackInfo and AlbumInfo #2654
Conversation
TrackInfo and AlbumInfo inheritate from class ItemInfo, which behaves like a dict and is backward compatible regarding atributes management. apply_metadata function has been updated consequently, favouring dict like item access more than attributes
|
@gabrielhdt, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sampsyo, @jrobeson and @dosoe to be potential reviewers. |
Updated calls to TrackInfo and AlbmInfo using keywords only arguments, implemented reduce methods for TrackInfo and AlbumInfo for copy.deepcopy (use also track_id and album_id) beetbox#1547
On the way to become generic. Used dictionnaries to map item attributes to ItemInfo keys
beets/autotag/hooks.py
Outdated
|
|
||
| def __reduce__(self): | ||
| """Used by :py:func:`copy.deepcopy`""" | ||
| return self['album_id'] |
There was a problem hiding this comment.
That's what I was looking for! How did you know this is the missing part for deepcopy?
There was a problem hiding this comment.
I looked into copy.deepcopy ... you can see it on the pickle doc too:
https://docs.python.org/2/library/pickle.html#object.reduce
beets/autotag/__init__.py
Outdated
| 'artist_sort': 'artist_sort', | ||
| 'artist_credit': 'artist_credit', | ||
| 'mb_artistid': 'artist_id', | ||
| } |
There was a problem hiding this comment.
Why not just make a loop over all tags? Then when one adds a tag he doesn't need to add something here.
There was a problem hiding this comment.
What you suggest must be eventually done. For the moment I adapted the current process to behave in a 'generic-friendly' way. I will come back to apply_metadata after having edited the track_info function of mb.py to make it capable of copying tags from recording object to TrackInfo.
|
Thanks! Then I will just kill my own try (that failed on |
ItemInfo populated using a model (the alias dict) which is parsed to automatically set attributes.
|
It seems that perfect flexibility (i.e. dumping directly musicbrainz data to tags) isn't possible, due to the recursive architecture of musicbrainz relations (relations that have relations, which can be seen in musicbrainzngs dict as they are recursive) which can't be applied to the media tags. Therefore, I chose to base the tagging process on the parsing of a mapping. The mapping reproduces the architecture of the musicbrainzngs dict and maps the musicbrainzngs dict key to the tag key (the ItemInfo attribute/key). Flexibility is improved as the mapping should eventually be stored in a json or yaml file, which will have to be updated to follow musicbrainz database schemes updates, one shouldn't need to edit python code to add an attribute, only update model. Tags that need processing are treated separately (e.g. track index). |
Old behaviour must be reproduced (to avoid failures in unit tests)
beets/autotag/__init__.py
Outdated
| 'arranger': 'arranger', | ||
| 'media': 'media', | ||
| 'disctitle': 'disctitle', | ||
| } |
There was a problem hiding this comment.
Hi! Thanks for all the work, it looks awesome. A question: will it be necessary for each new tag to add a corresponding line to the mapping?
There was a problem hiding this comment.
Yes it will. But that shouldn't be a problem, since the only task will eventually be to add a line to a json/yaml file.
Special function have to be implemented, database management must be improved (e.g. dynamically adding columns)
|
I'm wondering (I didn't try it out) how it works if there are several albumartists: what is currently done is that the names of all the artists are appended in a string, does something similar happen here or does he just take the last one? |
|
@gabrielhdt is there any interest in continuing this? |
|
Hi @jtpavlock, I don't really know, is it still relevant, and aren't there too many merge conflicts? On the part of the remaining code, if I remember correctly, we have to tinker with the database. I don't think I'll continue on this (even though I still use the software, so I'm still sort of interested). But I think that we should agree first on what we want: it has emerged from discussions with @dosoe that the needs differ in function of the people (which is normal), while I was more targeting for a system that mirrors musicbrainz' data on the files I have, @dosoe wanted to be able to add musics that should not be referenced into musicbrainz. |
|
Ah, that's nice, thank you @dosoe! I guess this pull request is really out of date then. |
|
Now that flexible tags have been implemented, it's much easier to do. |
|
Awesome, in that case I'll close this in favor of #3568 Thanks for your work on this @gabrielhdt! Sorry we never got to see it all the way through. |
TrackInfo and AlbumInfo inheritate from class ItemInfo, which behaves
like a dict and is backward compatible regarding attributes management.
apply_metadata function has been updated consequently, favouring dict
like item access more than attributes. Apply_metadata isn't general yet.
#1547