Skip to content

Improvement for script#1

Open
mewforest wants to merge 2 commits intosmtchahal:masterfrom
mewforest:master
Open

Improvement for script#1
mewforest wants to merge 2 commits intosmtchahal:masterfrom
mewforest:master

Conversation

@mewforest
Copy link

  • Fixes playlist duplicates (old version creates multiple playlists chucked to 100 tracks with the same name)
  • Fixes BadRequest error, when search string is too big for Spotify API
  • Adds progress bar (shows time elapsed and time remaining

Sergey Strukov added 2 commits January 14, 2023 19:45
Copy link
Owner

@smtchahal smtchahal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the late review. I just saw this PR, it somehow missed my inbox.

Looks like a solid improvement overall! Just needs a few things fixed, see the added comments.

@@ -1,2 +1,3 @@
python-dotenv>=0.13.0,<=0.13.99
spotipy>=2.19.0,<=2.19.99
tqdm>=4.64.1 No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tqdm>=4.64.1
tqdm>=4.64.1,<=4.64.99

Would recommend limiting package version to be the latest patch version as done with previous packages -- we don't want it to suddenly upgrade to say v5.0.0 and break functionality.


def _run_txt(self):
with open(self.songs) as songs_file, open('failed.txt', 'w') as failed_file:
playlist = self.sp.user_playlist_create(user=self._get_user_id(), name=self.playlist, public=False)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.playlist does not exist if the destination is 'library', so the script fails when run with destination=library. Need to add a check for destination and pass in a default value (e.g. None) instead of self.playlist if the destination is library.

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.

2 participants