-
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 popularity tag #4102
Add popularity tag #4102
Conversation
beets/library.py
Outdated
@@ -511,6 +511,7 @@ class Item(LibModel): | |||
'releasegroupdisambig': types.STRING, | |||
'disctitle': types.STRING, | |||
'encoder': types.STRING, | |||
'popularity': types.INTEGER, |
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.
Wondering if we should use spotify_popularity
as the label. LastFm also provides popularity values.
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.
No problem, it should be changed now
Seems good overall to me! Can you also please add a changelog entry? |
Ok, that should be done |
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.
Thanks! Giving this a more thorough read a second time through, I see a couple of changes that would be useful. In particular, I think that the plugin should "own" this flexible attribute rather than having it baked into beets core. The plugin can provide the integer type for the field.
Also, to confirm, have you been able to try this feature out? That is, have you imported albums tagged with the spotify
plugin and witnessed the field being set correctly? It can be useful to see things working at least once (in the absence of tests).
Co-authored-by: Adrian Sampson <adrian@radbox.org>
…into Add-popularity
Looking pretty good overall… when you get a chance, can you please confirm that this works for you? |
I'm not sure how to test if it works or not |
I think a good way would be to set up the plugin and try using it to tag some music. |
Sorry I am not very familiar with this library. When I try tagging some music, it only provides the title and artist, even though it is marked as a recognized by Spotify. I am also unsure how to change the spotify plugin being used when I import beets to be the one with the relevant changes. |
Well, you'll want to install the source code you have modified (something like |
I am absolutely interested in testing it out, but not sure how to go about doing that :( |
The FAQ has instructions for installing beets from source, which might be a good place to start. |
Ok...so I tried it and here's the error that I am getting: PS C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\Scripts> .\beet import "C:\Users\Alok\Downloads\Test"
Error: MusicBrainz not reachable in get release by ID with query 'b59f6b6e-b6e0-405f-a80c-a1859942a86e'
Traceback (most recent call last):
File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.9_3.9.2032.0_x64__qbz5n2kfra8p0\lib\runpy.py", line 197, in _run_module_as_main
return _run_code(code, main_globals, None,
File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.9_3.9.2032.0_x64__qbz5n2kfra8p0\lib\runpy.py", line 87, in _run_code
exec(code, run_globals)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\Scripts\beet.exe\__main__.py", line 7, in <module>
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\ui\__init__.py", line 1278, in main
_raw_main(args)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\ui\__init__.py", line 1265, in _raw_main
subcommand.func(lib, suboptions, subargs)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\ui\commands.py", line 973, in import_func
import_files(lib, paths, query)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\ui\commands.py", line 943, in import_files
session.run()
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\importer.py", line 340, in run
pl.run_parallel(QUEUE_SIZE)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\util\pipeline.py", line 446, in run_parallel
raise exc_info[1].with_traceback(exc_info[2])
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\util\pipeline.py", line 311, in run
out = self.coro.send(msg)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\util\pipeline.py", line 193, in coro
func(*(args + (task,)))
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\importer.py", line 1376, in lookup_candidates
task.lookup_candidates()
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\importer.py", line 660, in lookup_candidates
autotag.tag_album(self.items, search_ids=self.search_ids)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\autotag\match.py", line 461, in tag_album
for matched_candidate in hooks.album_candidates(items,
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\plugins.py", line 573, in decorated
for v in generator(*args, **kwargs):
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\autotag\hooks.py", line 629, in album_candidates
yield from plugins.candidates(items, artist, album, va_likely,
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\plugins.py", line 384, in candidates
yield from plugin.candidates(items, artist, album, va_likely,
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\plugins.py", line 733, in candidates
albums = [self.album_for_id(album_id=r['id']) for r in results]
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beets\plugins.py", line 733, in <listcomp>
albums = [self.album_for_id(album_id=r['id']) for r in results]
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beetsplug\spotify.py", line 200, in album_for_id
track = self._get_track(track_data)
File "C:\Users\Alok\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\LocalCache\local-packages\Python39\site-packages\beetsplug\spotify.py", line 246, in _get_track
spotify_popularity=track_data['popularity'],
KeyError: 'popularity' |
Ok...I did some more digging and looks like the popularity information may require an extra API call per track.
That information is not available in the album API call. @rhlahuja indicated this in one of the closed threads. s/he was the last contributor to the Spotify plugin. |
Ok I can quickly implement a track api call when iterating through the album response then. This will give access to a lot more information about each track |
OK! As before, it would be great to hear about a demonstration that this has been tried and actually works. It also adds an extra request per track, I think. That seems like it could slow things down a lot. Do we know how much? If it is indeed much slower, maybe this should be an option (off by default)? |
Agreed on the track API call being optional (configurable). In addition, we need to be sure that this runs outside the importer with |
SO, I was able to run the updated code, but I did not see any new
|
@happiestbee Can you confirm if you getting the |
I believe it is working on my end
|
Indeed, I see those too. Looks like it is working
For some reason I am not able to find the musiclibrary.blb file to browse the database on my test machine. Can you also see if you get that info with |
@@ -243,6 +246,7 @@ def _get_track(self, track_data): | |||
medium_index=track_data['track_number'], | |||
data_source=self.data_source, | |||
data_url=track_data['external_urls']['spotify'], | |||
spotify_popularity=track_data['popularity'], |
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.
I think this should be item.spotify_popularity
so that the information is available in the items
table. I also think it may be a good idea to save id
while we are at it (similar to how MusicBrainz does it).
Maybe @sampsyo can confirm this before you make any changes.
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.
I think you are right, I will go ahead and make the changes.
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.
To clarify, is this what you mean?
def apply_item_metadata(item, track_info):
"""Set an item's metadata from its matched TrackInfo object.
"""
item.artist = track_info.artist
item.artist_sort = track_info.artist_sort
item.artist_credit = track_info.artist_credit
item.title = track_info.title
item.mb_trackid = track_info.track_id
item.mb_releasetrackid = track_info.release_track_id
item.spotify_popularity = track_info.spotify_popularity
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.
These changes have been made
Did you push those changes? I still see |
FWIW, changing beets/beets/autotag/__init__.py Line 92 in b8b74a7
|
So is this feature fine the way it is right now? |
But it is currently being added to the |
It is indeed; that's where flexible (rather than fixed) attributes live. |
So is this able to be merged? |
Well, I would love to see a little more discussion (or implementation) on this point from above:
|
I'm not sure where to implement this option since the autotagger is not a subcommand of the plugin |
It doesn't look like it is adding more calls right now. Currently, it is just gathering the information available at the time of import (which was available even earlier but was not stored). |
I did need to add a new call since the track info being passed was actually the track info contained in an album call, which does not contain all info such as the popularity of the track |
While we are at it...can we also save the track_id (maybe part of |
So, I tested this a little more and it looks like the |
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Get popularity tag for tracks from track api in Spotify plugin
Fixes #4094 .
Makes use of the popularity information for tracks
To Do
docs/
to describe it.)docs/changelog.rst
near the top of the document.)