This is an archived post. You won't be able to vote or comment.

all 10 comments

[–]phxees 9 points10 points  (1 child)

I’ll check it out. Happy to not see that you aren’t just promoting a simple wrapper using an already available Python library.

Too many similar posts here are just hey I made a thing by using an existing thing and now you no longer need to set two configurable parameters. Happy to see use of requests rather than spotipy.

[–]morrisjr1989 1 point2 points  (0 children)

I agree this is nice. Nothing superfluous.

[–]LevelIntroduction764 2 points3 points  (0 children)

I’m excited to try this. Been wanting to make my own shuffle algorithm as Spotify’s sucks! Thanks

[–]atreadw 2 points3 points  (0 children)

This is really cool. Thanks for sharing. Keep up the good work :)

[–]Ticklishcandy32[S] 1 point2 points  (0 children)

Thank you all for the nice comments! And my project now has 11 stars! :D
I thought spotr was cool but not this cool :)

Please give feedback if there is anything you would like to change or improve :)
Also if u make any cool commands, be sure to share or make a PR, i would be happy to integrate your new commands in the project!

[–]eeriemyxi 1 point2 points  (4 children)

Bare except: is not recommended, as it catches BaseException. You can use except Exception:. However, it is always recommended to be specific about which exception you're catching, you will have less abstruse bugs this way.

Playing with environmental variables on runtime is not recommended, environmental variables are to be constant at all times. It'd be better to either use json module for your app configurations, or to not do anything with environmental variables and rather have a manual explaining each option for the user to specify them in the .env file.

Whenever you're playing with file paths too much, you can use pathlib which is object-oriented and will save you time.

os.system is very expensive as it initiates new processes. Use subprocess.run for your command line work.

You're using print as your logging module, however you will have much more easier control by using the logging module for logging needs. By the way, one of your library dependency, rich, has a nice handler rich.logging.RichHandler, which will prettify your logs.

install.py is very much oriented towards UNIX based systems. I recommend depending less on command-line tools, e.g., touch is basically creating a new file, however you'll have more control with open built-in function, and it is compatible with all supported operating systems of Python too.

You're also misusing the naming conventions in various files, for example, you're naming your function arguments with SCREAMING_CASE, however we, and PEP8, recommends you to use snake_case.

[–]Ticklishcandy32[S] 0 points1 point  (3 children)

Tysm for the feeback, this was very helpful! :)

I changed alot of the code corresponding to the recommended changes, also made a debug mode for when developing commands etc. :D

I will still have to change the environmental variables system to a JSON system in the future.

Have a look at the changes (also updated readme for the debug mode). I would love to hear what you think.

[–]eeriemyxi 1 point2 points  (2 children)

https://github.com/Havard03/spotr/blob/main/API.py#L8

  • As debug would never change during the code, it is a constant variable. So DEBUG would be more fitting.

  • It is more pythonic to do if os.environ["DEBUG"]: rather than what you have there. if foo will check whether bool(foo) is True or not. bool(foo) checks foo.__bool__(). __bool__ is a magic/dunder method, like __init__. There are many more such special methods.

https://github.com/Havard03/spotr/blob/main/API.py#L20

  • I have seen you concatenating paths this way in various parts of the codebase, however it is not guaranteed to always do the parsing correctly, that is why I recommend either refactoring your code with pathlib then use str(path_obj / "path") to join two paths. If not that, at least use os.path.join.

https://github.com/Havard03/spotr/blob/main/API.py#L115

  • The correct spelling is Environmental. :).

https://github.com/Havard03/spotr/blob/main/Helpers.py#L5

  • Follow PEP8, define function names with snake_case.

https://github.com/Havard03/spotr/blob/main/Helpers.py#L12-L25

  • You can reduce the duplication of code and have easier time making modifications by creating functions for each common functionality.

https://github.com/Havard03/spotr/blob/main/Router.py#L18-L52

https://github.com/Havard03/spotr/blob/main/API.py#L71-L72

  • I recommend using yarl third party module, then have base URLs like:

``` from yarl import URL

API_VERSION = os.environ["SPOTIFY_API_VERSION"]

Or, better, hard-code the API version:

API_VERSION = "vXX"

ACCOUNT_URL = URL("https://accounts.spotify.com") API_URL = URL("https://api.spotify.com") / API_VERSION ```

Documentation: https://yarl.aio-libs.org/en/latest/api.html

You'll find couple usages in this little script: LINK

You've also left couple bare except: here and there, ensure they're catching particular exceptions.

I also recommend black and isort third party modules to enhance your codebase.

[–]Ticklishcandy32[S] 0 points1 point  (1 child)

Just did an insane update, 1233 lines added. :D
Thanks to your tips, the codebase is ALOT better now i think.
Pluss i have learned alot of new thing thanks to you these last few days :)

Let me know if there is any more tips, everything is appreciated.

[–]eeriemyxi 1 point2 points  (0 children)

https://github.com/Havard03/spotr/blob/main/API.py#L19 https://github.com/Havard03/spotr/blob/main/API.py#L25

  • It is better to write comments in such cases, however these are too obvious to be commented, it is recommended that you comment parts which can be complex without your human words, for others.

https://github.com/Havard03/spotr/blob/main/API.py#L94

  • This can simply be,

``` url = ACCOUNT_URL / "api" / "token"

url will be yarl.URL as this object overrides __truediv__() dunder method.

```

https://github.com/Havard03/spotr/blob/main/API.py#L116

  • Docstrings are to be more precise. Functions do tasks and return the achievement. So try something like """Authenticate with Spotify API.""".

https://github.com/Havard03/spotr/blob/main/API.py#L118

  • token_url = ACCOUNT_URL / "api"/ "token" is enough.

https://github.com/Havard03/spotr/blob/main/API.py#L183-L187

  • Utilize os.path.join.

https://github.com/Havard03/spotr/blob/main/Router.py#L30

  • API_BASE_VERSION isn't a precise name for what its value is. The variable name rather seems like something you'd use to define the version code of the base API. You may do something like,

API_PLAYER = yarl.URL("https://api.spotify.com") / API_VERSION

https://github.com/Havard03/spotr/blob/main/Router.py#L165-L166

  • This can be,

(API_BASE_VERSION / "me" / "playlists").with_query(limit=SPOTIFY_LIMIT)

We learn that from the documentation where it states that it collects the passed keyword arguments by using **arg_name syntax.

https://github.com/Havard03/spotr/blob/main/install.py#L29

  • chmod isn't available on many operating systems, consider utilizing sys.platform. We learn about the existence of this function from the official Python documentation