13
\$\begingroup\$

I want to backup my GitHub repositories since there is news about blocking GitHub in India. I wrote Python code to automate this and would like for it to be reviewed.

import os
import requests

API_TOKEN='xxxxx'
#from https://github.com/settings/applications

api_repo='https://api.github.com/user/repos'

url = '%s?access_token=%s' % \
(api_repo,API_TOKEN,)

r = requests.get(url).json()
git_urls = [repo['git_url'] for repo in r]
for i in git_urls:
  os.system("git clone "+i) 
\$\endgroup\$

1 Answer 1

13
\$\begingroup\$

requests is third-party, so (per the style guide) there should be a blank line in the imports:

import os

import requests

api_repo is constant, so should be API_REPO. There should also be spaces around the = in assignments:

API_REPO = 'https://api.github.com/user/repos'

Explicit line continuation isn't very Pythonic, especially when the line is short enough anyway:

url = '%s?access_token=%s' % \
(api_repo,API_TOKEN,)

Also, there should be spaces after commas (and I wouldn't bother with the trailing one). This would be better written as:

url = '%s?access_token=%s' % (API_REPO, API_TOKEN)

r and i aren't very good variable names. I would use req_json and git_url.


You are mixing % string formatting with + concatenation. You should at least be consistent, and I would use the more modern str.format:

os.system("git clone {}".format(git_url)) 

You should also be consistent with string literal quotes. Per the style guide:

In Python, single-quoted strings and double-quoted strings are the same. This PEP does not make a recommendation for this. Pick a rule and stick to it. When a string contains single or double quote characters, however, use the other one to avoid backslashes in the string. It improves readability.


In all, I would probably have written it as:

import os

import requests

API_TOKEN = "..."
API_URL = "https://api.github.com/user/repos"

url = "{}?access_token={}".format(API_URL, API_TOKEN)
req_json = requests.get(url).json()

for repo in req_json:
    os.system("git clone {}".format(repo["git_url"]))
\$\endgroup\$
5
  • 1
    \$\begingroup\$ There is a stray access_token parameter in API_URL which should be removed I think. \$\endgroup\$
    – ChrisWue
    Commented Jan 1, 2015 at 19:05
  • \$\begingroup\$ Where in the style guide does it talk about blank lines between imports? I just read over it but I seem to have missed that. \$\endgroup\$
    – David Z
    Commented Jan 2, 2015 at 4:10
  • 1
    \$\begingroup\$ pylint pep8 doesn't find it, but I've been doing something similar; std imports [\n\n] lib imports [\n\n] proj imports \$\endgroup\$
    – Wyrmwood
    Commented Jan 2, 2015 at 4:16
  • 2
    \$\begingroup\$ @DavidZ: python.org/dev/peps/pep-0008/#imports Section "Imports", second bullet, second paragraph. \$\endgroup\$
    – mike3996
    Commented Jan 2, 2015 at 6:47
  • 2
    \$\begingroup\$ You forgot mixing quote styles. Even your example has a single-quoted string in one place. \$\endgroup\$
    – user688
    Commented Jan 2, 2015 at 10:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.