UX of rating songs with Mobileclient

[thebigmunch]

There has been plenty of confusion in the past (and likely present) about Mobileclient.change_song_metadata not having the same feature set as Webclient.change_song_metadata did.

I'm wondering if adding a rate_song method to be the frontend to rating a song might be an improvement over documenting that it can only change rating. The call could look something like rate_song(song_dict, rating). If deemed proper, change_song_metadata could be deprecated for future removal ala our discussion of add_store_track/add_store_tracks.

Semantically, this would match Google's UX which separates these concerns in their frontends. The web UI has rating in the song list but not in the 'Edit Song Info' form, and the Android app has rating while playing but no info editing at all.

I thought about having it take either a song_dict or a song_id. But, unlike library songs, store songs require at least a trackType field as well as the id field and rating in the dictionary, so we'd need to have a check for store vs library and then call out to Mobileclient.get_track_info for store tracks. This adds extra layers of abstraction/processing that I don't think are worthwhile in the end. I'm just throwing it out there to see if anyone would actually be significantly benefited by it.

Edit: Fun fact: Rating a track while playing in the Android app actually uses the activity/recordrealtime endpoint.

[simon-weber]

This sounds smart: change_song_metadata doesn't make much sense anymore.

My guess is that a song_dict would be the best interface. We could just document the minimum requirements of the dict for folks who want to call it but only have an id and type.

[thebigmunch]

Before I send in this PR, I just need some confirmation on one of those things I obsess about ^_^

I believe rate_song(song, rating) to be the ideal function signature. But I have a couple other possibilities in my head.

1. rate_songs(songs, rating) where we use accept_singleton ala change_song_metadata. But I don't believe there's really a good use for sending a list of songs unless, like change_song_metadata, you have people add the rating key to the dicts instead of passing the rating as an arg (see 2 for my thoughts on this). You could also simply do rate_song(song, rating) in the loop you would have used to build the list in the first place or alongside building the list if you actually need the list later.

2. rate_songs(songs) where it's basically just a name change of change_song_metadata. The benefit of this is familiarity and ease of porting; it's a simple name change. But I believe requiring users to manually add a rating key to a song dict before passing it to the function to be poor usability. This bleeds too much of the implementation into the user interface and is a hassle for people who don't have pre-rated songs in a local DB of some sort. And, to be honest, I believe the interface for change_song_metadata probably should have been change_song_metadata(song, **kwargs) where kwargs are the key: value pairs that need updating. Also, I find not passing a rating to a function called rate_foo to be pretty bad semantics.

In my opinion, rate_song(song, rating) provides better usability and semantics while keeping as much magic/implicit conversions out of the implementation. If anyone has any reason they think there's a better interface than rate_song(song, rating), please post it.

[simon-weber]

I'm 👍 on rate_song(song, rating). I'm fine with rate_songs(songs, rating) too, if only because it'd be less calls to rate a bunch of songs (though I'm not sure how often that happens).

the interface for change_song_metadata probably should have been change_song_metadata(song, **kwargs)

Haha, yup. Here's a super old issue about it: #30.

[thebigmunch]

if only because it'd be less calls to rate a bunch of songs (though I'm not sure how often that happens).

Yeah, I'm really wondering how often that happens as well. As for being less calls, I don't think it's actually possible to rate multiple songs at once in the Google Play Music app (Android, at least), so I imagine that's no different than what happens with the official client anyway. I'm not sure if the calls happening much faster in a loop in Python than manually rating multiple songs in the Android app would have any effect, though.

[thebigmunch]

I just rated ~50 songs in a for loop without issue. But the time-to-complete definitely takes a hit making multiple calls. I'll probably run some tests to see how much of a difference there is. In the meantime, I'd love to hear if anyone rates large numbers of songs at once. Perhaps @chrisnorman7 would have an opinion on this, since he previously had an issue open about rating songs from his local DB.

[thebigmunch]

So, performance-wise, it looks like rate_songs(songs, rating) might be what we have to go with. The baseline for a single call in my tests was ~3 seconds. Calling rate_song in a for loop over 20 songs hit ~8 seconds while 60 songs hit ~17 seconds. That might be too much of an increase even for an edge case.

[chrisnorman7]

Hi mate,
Not rating multiple songs here, and I let the rating block, so my users
can't run through and rage-rate without some script or other.

I'm perfectly happy with the current system, and I fixed my bugs
according to your suggestions.

Thanks for all you do, and take care.

On 06/09/16 21:02, thebigmunch wrote:

I just rated ~50 songs in a for loop without issue. But the
time-to-complete definitely takes a hit making multiple calls. I'll
probably run some tests to see how much of a difference there is. In
the meantime, I'd love to hear if anyone rates large numbers of songs
at once. Perhaps @chrisnorman7 https://github.com/chrisnorman7 would
have an opinion on this, since he previously had an issue open about
rating songs from his local DB.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#480 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AENM6QJ-dNub1VnHTrrSOXCSIVUrdApDks5qncbigaJpZM4JfBNk.