[PR #1049] [CLOSED] Jonathan Huston's 12-04-2023 code review of /examples directory #1137

New issue

Closed

opened 2026-02-28 00:03:47 +03:00 by kerem · 0 comments

kerem commented

2026-02-28 00:03:47 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/spotipy-dev/spotipy/pull/1049
Author: @jonathan-huston
Created: 12/5/2023
Status: ❌ Closed

Base: master ← Head: code_review_JH

📝 Commits (1)

8358b26 Jonathan Huston's 12-04-2023 code review of /examples/ directory

📊 Changes

11 files changed (+78 additions, -5 deletions)

View changed files

📝 examples/add_a_saved_album.py (+8 -1)
📝 examples/add_a_saved_track.py (+4 -0)
📝 examples/add_tracks_to_playlist.py (+4 -0)
📝 examples/artist_albums.py (+10 -0)
📝 examples/artist_discography.py (+21 -2)
📝 examples/artist_recommendations.py (+10 -0)
📝 examples/audio_analysis_for_track.py (+11 -0)
📝 examples/audio_features.py (+2 -0)
📝 examples/change_playlist_details.py (+1 -0)
📝 examples/client_credentials_flow.py (+2 -0)
📝 examples/follow_playlist.py (+5 -2)

📄 Description

I performed an initial code review the examples directory. I started with add_a_saved_album.py and went in alphabetical order until follow_playlist.py. Here are my findings.

Almost all the example scripts work as intended and are generally easy to follow. This is an excellent directory to help people get started with Spotipy.
follow_playlist.py did not run since no authorization scope was set. Once this was corrected, the script ran as intended. I would verify the other scripts have the correct authorization scope set (authorization scopes can be found in the Spotify Developer reference guide when needed. See here for example.
client_credentials_flow.py was redundant. Since the examples directory hasn't been updated in a while, it might be worth doing a pass of each example script and ensure that it is still relevant.
There was inconsistency in some of the conventions use for the different example scripts. For example, add_a_saved_album.py uses the argparse library to retrieve arguments, while audio_features.py does not, and will default to a hardcoded argument if none is provided (or simply run with no arguments). I find the former of these more clear, but either way I would make sure this is done the same way across the examples. Similarly, some examples have a name==main block while others do not. Including this block is probably better practice, even if these scripts don't get imported anywhere.
Per PEP8 standards, there was a lack of module and method docstrings. I understand that in this case it's clear what method is being demonstrated, and that the methods themselves are already well documented in client.py, but it's still good practice to include them.

See comments in files below.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/spotipy-dev/spotipy/pull/1049 **Author:** [@jonathan-huston](https://github.com/jonathan-huston) **Created:** 12/5/2023 **Status:** ❌ Closed **Base:** `master` ← **Head:** `code_review_JH` --- ### 📝 Commits (1) - [`8358b26`](https://github.com/spotipy-dev/spotipy/commit/8358b26870281a42f4959facb1fd394dfd0e10c1) Jonathan Huston's 12-04-2023 code review of /examples/ directory ### 📊 Changes **11 files changed** (+78 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `examples/add_a_saved_album.py` (+8 -1) 📝 `examples/add_a_saved_track.py` (+4 -0) 📝 `examples/add_tracks_to_playlist.py` (+4 -0) 📝 `examples/artist_albums.py` (+10 -0) 📝 `examples/artist_discography.py` (+21 -2) 📝 `examples/artist_recommendations.py` (+10 -0) 📝 `examples/audio_analysis_for_track.py` (+11 -0) 📝 `examples/audio_features.py` (+2 -0) 📝 `examples/change_playlist_details.py` (+1 -0) 📝 `examples/client_credentials_flow.py` (+2 -0) 📝 `examples/follow_playlist.py` (+5 -2) </details> ### 📄 Description I performed an initial code review the examples directory. I started with add_a_saved_album.py and went in alphabetical order until follow_playlist.py. Here are my findings. - Almost all the example scripts work as intended and are generally easy to follow. This is an excellent directory to help people get started with Spotipy. - follow_playlist.py did not run since no authorization scope was set. Once this was corrected, the script ran as intended. I would verify the other scripts have the correct authorization scope set (authorization scopes can be found in the Spotify Developer reference guide when needed. See [here](https://developer.spotify.com/documentation/web-api/reference/change-playlist-details) for example. - client_credentials_flow.py was redundant. Since the examples directory hasn't been updated in a while, it might be worth doing a pass of each example script and ensure that it is still relevant. - There was inconsistency in some of the conventions use for the different example scripts. For example, add_a_saved_album.py uses the argparse library to retrieve arguments, while audio_features.py does not, and will default to a hardcoded argument if none is provided (or simply run with no arguments). I find the former of these more clear, but either way I would make sure this is done the same way across the examples. Similarly, some examples have a __name__==__main__ block while others do not. Including this block is probably better practice, even if these scripts don't get imported anywhere. - Per PEP8 standards, there was a lack of module and method docstrings. I understand that in this case it's clear what method is being demonstrated, and that the methods themselves are already well documented in client.py, but it's still good practice to include them. See comments in files below. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>