Skip to content
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

Supports the use of str in the recommendations function argument and Added Implemented url_to_id function to extract id from url #1074

Closed
wants to merge 0 commits into from

Conversation

CureSaba
Copy link

@CureSaba CureSaba commented Feb 22, 2024

Supports the use of str in the recommendations function argument

why I did

This is to solve the problem that "," is inserted between characters by .join when str type is specified as a factor of the recommendations function.

Added url_to_id function to util module You can use this to extract id from url

why i did

The spotipy document says that URL input can be used, but the official API document says that it cannot be used, so I created a conversion function.

@CureSaba CureSaba changed the title Supports the use of str in the recommendations function argument Supports the use of str in the recommendations function argument and Added url_to_id function to util module You can use this to extract id from url Mar 5, 2024
@CureSaba CureSaba changed the title Supports the use of str in the recommendations function argument and Added url_to_id function to util module You can use this to extract id from url Supports the use of str in the recommendations function argument and Added Implemented url_to_id function to extract id from url Mar 5, 2024
@Mews
Copy link

Mews commented May 1, 2024

A url to id function would be goated

@dieser-niko
Copy link
Member

There's already a function that converts URIs/URLs to IDs called Spotify._get_id.

But imo the url_to_id function from CureSaba would still be useful for programmers, since the already implemented function is intended for internal use within the Spotify module. It's still not perfect though. For example there are other types of IDs like playlists, episodes, albums, artists, etc.
But currently the function would only work for tracks.
There's already a regex string for URIs and URLs called _regex_spotify_uri and _regex_spotify_url.

Also the parameter url is implying that a string is being expected, but the input has to be a list of strings instead.
The variable name a should be renamed to something more reasonable and the docstring for the function is outside of the function.

About the implementation in client.py. I'm not sure this is a good idea. Checking the Spotify API shows that Spotify specifies the parameter as being of type list, and the name of the parameter (which is plural) also implies that it should be a list. I understand that this change might make things easier, but adding brackets to an argument shouldn't be too hard either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants