Little command line video grabber

Question

I made a little Python3 script to grab video clips from a website for viewing off-line later and would like your critique, advice, tips & tricks and/or learning resources to create better code. (Project on GitHub)

#!/usr/bin/env python3
''' phgrab.py: clip downloader. '''

import os
import sys
import argparse
import urllib.request
import bs4


def reporthook(blocknum, blocksize, totalsize):
    ''' Define the reporthook for url-retrieval
    (displays info while grabbing). '''
    readsofar = blocknum * blocksize
    if totalsize > 0:
        percent = readsofar * 1e2 / totalsize
        display = "\r%*d / %d %5.0f%%" % (
            len(str(totalsize)), readsofar, totalsize, percent)
        sys.stdout.write(display)
        if readsofar >= totalsize:
            sys.stdout.write("\n")
    else:   # total size unknown
        sys.stderr.write("read %d\n" % (readsofar,))


PARSER = argparse.ArgumentParser(
    description="a commandline clip downloader.")
PARSER.add_argument('clipcode', nargs='+',
                    help="a ten-digit clipcode.")
PARSER.add_argument('-p', '--proxy',
                    help="A http proxy to use; e.g. http://localhost:8000")

PROXY = PARSER.parse_args().proxy
CLIPCODES = PARSER.parse_args().clipcode

if PROXY:
    print("Grabbing through http proxy: %s\n" % PROXY)
    os.environ['http_proxy'] = PROXY

for i in CLIPCODES:
    if not i.isdigit() or not len(i) == 10:
        sys.exit(
            "'\033[1m%s\033[0m' is not a valid ten-digit clipcode," % i,
            "please check your input and try again.")

for i in CLIPCODES:
    soup = bs4.BeautifulSoup(
        urllib.request.urlopen("http://somesite.com/%s" % i))
    title = soup.find("h1", class_="video-title").string
    href = soup.find("a", class_="download button")['href']
    print("Grabbing \033[1m%s\033[0m:" % i)
    local_filename, headers = urllib.request.urlretrieve(
        href, '%s - %s.mp4' % (i, title), reporthook)
    print("\033[1m", local_filename, "Done\033[0m")

Answer 1

A bug

This just won't work:

sys.exit(
    "'\033[1m%s\033[0m' is not a valid ten-digit clipcode," % i,
    "please check your input and try again.")

sys.exit takes a single integer parameter to use as the status code of the program, so that callers can check if the program terminated normally or failed. I suppose you wanted to print instead and then sys.exit(1).

Move all code to methods

Don't keep code in the global namespace. A good practice is to move everything to methods, and call them from inside a if __name__ == "__main__": guard, like this:

def parse_args():
    # arg parser code

def main():
    args = parse_args()
    proxy = args.proxy
    clipcodes = args.clipcode

if __name__ == "__main__":
    main()

This way it will be possible to import the module into a larger app later. Another advantage is that it doesn't pollute the global namespace, which could lead to obscure bugs.

Use print() if you want to print

Printing with sys.stdout.write is very unusual and tedious. Using print() is more common and simple. If you don't want to print a newline, use the end='' flag, for example:

print(message, end='')

Naming

i is really a very poor name in this loop:

for i in CLIPCODES:
    if not i.isdigit() or not len(i) == 10:
        # ...

It would be better to use something more descriptive, for example clipcode.

Another thing here, not len(i) == 10 is a very odd way of writing len(i) != 10.

---

It's very unusual to see ALL_CAPS for variable names that are not global constants. For example DEFAULT_PROXY_PORT = 8080 would make a fine global constant, but these look really odd:

PARSER = argparse.ArgumentParser(...)
CLIPCODES = PARSER.parse_args().clipcode