search

pavelkomarov / exportify

Export Spotify playlists using the Web API. Analyze them in the Jupyter notebook.
https://exportify.net
MIT License
210 stars 24 forks source link

extract additional album info #60

Closed ChinoUkaegbu closed 2 months ago

ChinoUkaegbu commented 2 months ago

Fixes #32

In addition to returning the album labels, also returns total number of tracks in the album as well as the album type because why not?

Also turns out Spotify's API also lets you get genres for albums which I think is more specific than Artist's genres which is what was implemented. Unfortunately for all the albums I tried it with, it just always returned an empty list (even when I tried on their website) so either it's a bug on Spotify or idk

Which sucks because it would also have cut down the amount of requests we were doing since we won't have needed to query the artist's endpoints too.

ChinoUkaegbu commented 2 months ago

Also lmk if I should get rid of the extra fields since only label was initially asked but I doubt the additional fields are adding much delays since it's all retrieved either way.

pavelkomarov commented 2 months ago

Regarding empty lists for genres: Their Web API is definitely imperfect! I discovered an issue with it the other day https://github.com/pavelkomarov/exportify/issues/56. You may be able to open a forum post and get it addressed.

pavelkomarov commented 2 months ago

I feel like there's still potentially scope in this issue to make certain fields selectable, but then again that may be total overkill; maybe we should just give all the fields all the time and not worry the user with a choice they mostly don't care about. It's easier to wait a couple more seconds and ignore fields you don't need than to think through which ones you do or discover some you wanted actually weren't fetched because they were part of another group you deselected.

Philosophy of simplicity. It's why I got rid of the paginator. Related story: I once had an internship on Microsoft's Outlook team and discovered a bunch of PMs trying to think through new, additional features for the app. I told them it was far too cluttered and they should be looking for ways to remove features. Didn't win me friends, but I'm proud of myself for saying what needed to be said.

@gregsadetsky, do you have an opinion? We can always merge this and make selectability its own follow-on issue, if my instincts are wrong and it turns out to be in demand.

pavelkomarov commented 2 months ago

I just pulled your branch and tried to Export All, and I'm getting rate limiting errors and undefined is not an object errors.

I think this issue is both genre_promise and album_promise are solely dependent upon data_promise, so they're happening concurrently. You need a Promise.all([data_promise, genre_promise]).

I would hop on your branch and just fix it, but I don't think I have permissions on your fork to do that. Maybe I can merge to a feature branch on my own repo and then merge that? Or maybe there's another way https://tighten.com/insights/adding-commits-to-a-pull-request/

pavelkomarov commented 2 months ago

I've reported the empty list issue on the Spotify dev forum. According to a months-old forum post that I link therein, the workaround is fetching genres from artists. Unclear whether that is intentional behavior.

ChinoUkaegbu commented 2 months ago

I just pulled your branch and tried to Export All, and I'm getting rate limiting errors and undefined is not an object errors.

I think this issue is both genre_promise and album_promise are solely dependent upon data_promise, so they're happening concurrently. You need a Promise.all([data_promise, genre_promise]).

I would hop on your branch and just fix it, but I don't think I have permissions on your fork to do that. Maybe I can merge to a feature branch on my own repo and then merge that? Or maybe there's another way https://tighten.com/insights/adding-commits-to-a-pull-request/

my bad, i was very much asleep during this but glad you figured it out! i can add more comments to #62 but i do agree with the simplicity thing. with the way it's structured we already get most of the data in the first wave. the whole selection thing is for performance issues but the only way it makes an improvement is if users do not select genre and record label to be exported (i'm terrible at probability but it does seem like unlikely fields to deselect for no reason...unless you somehow include that deselecting those particular fields could improve performance in which case there's some sort of incentive to do so). from a (solely) usability perspective, i think the option to select fields to include is good though but might be worth it to have this in discussions.

gregsadetsky commented 1 month ago

sorry for not chiming in earlier, had some crazy weeks. thanks a LOT! I see now the labels in the csv output and that's fantastic! thank youuu!!