-
Notifications
You must be signed in to change notification settings - Fork 959
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
Formatted strings #1178
base: master
Are you sure you want to change the base?
Formatted strings #1178
Conversation
@haviv1idan you can also check out the PR. I've included some of your changes, but not all. For some, I have added a comment to your PR (#1165) |
Why mix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dieser-niko Very nice job.
Just a few suggestions to keep consistency in code
@@ -145,7 +145,7 @@ class SpotifyClientCredentials that can be used to authenticate requests like so | |||
playlists = sp.user_playlists('spotify') | |||
while playlists: | |||
for i, playlist in enumerate(playlists['items']): | |||
print("%4d %s %s" % (i + 1 + playlists['offset'], playlist['uri'], playlist['name'])) | |||
print("{:4d} {} {}".format(i + 1 + playlists['offset'], playlist['uri'], playlist['name'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dieser-niko I think we need to be consistent in our code.
Most of files are using f-string instead of format.
You can replace this code to this:
print(f"{i + 1 + playlists['offset']:4d} {playlist['uri']} {playlist['name']}")
logger.debug('Sending {} to {} with Params: {} Headers: {} and Body: {!r} '.format( | ||
method, url, args.get("params"), headers, args.get('data'))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
'HTTP Error for {} to {} with Params: {} returned {} due to {}'.format( | ||
method, url, args.get("params"), response.status_code, msg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
"Sending POST request to {} with Headers: {} and Body: {!r}".format( | ||
self.OAUTH_TOKEN_URL, headers, payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
"Sending POST request to {} with Headers: {} and Body: {!r}".format( | ||
self.OAUTH_TOKEN_URL, headers, payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
"Sending POST request to {} with Headers: {} and Body: {!r}".format( | ||
self.OAUTH_TOKEN_URL, headers, payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
"Sending POST request to {} with Headers: {} and Body: {!r}".format( | ||
self.OAUTH_TOKEN_URL, headers, payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
"Sending POST request to {} with Headers: {} and Body: {!r}".format( | ||
self.OAUTH_TOKEN_URL, headers, payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same here. replace format with f-string
Yeah honestly I don't know what went through my mind. I guess I preferred to use .format() because it would be easier to read, but I agree, consistency would actually be better. Currently I don't have the time to fix, expect activity in the next month. |
closes #1159
There are some capitalisations in there as well.
Also, I had to change the playlist in
examples/follow_playlist.py
to a user playlist due to the recent API changes.