I have created a simple program at https://github.com/Epic0ne/magpi-downloader in Python which downloades all issues of the MagPi magazine by parsing this web page for links ending in .pdf, then downloading them using the urllib module. It also supports command line options, using the argparse module:$ ./magpi_downloader.py -husage: magpi_downloader.py [-h] [-q] [-r] [--view] [-t FILETYPE] [-a] [-i]                           [-e]                           DIR [REMOTE_DIR]Download issues of the MagPi magazinepositional arguments:  DIR               The directory to install into.  REMOTE_DIR        The directory to fetch from. Files must be links on that                    page. Works best with Apache servers with the default                    directory listing. Default: http://www.raspberrypi.org                    /magpi-issues/optional arguments:  -h, --help        show this help message and exit  -q, --quiet       Silence progress output  -r, --reinstall   Reinstall all issues  --view            List the issues available for install  -t FILETYPE       The extension of the files to download. Default: pdf  -a, --all         Install all files with the right extension.  -i, --issues      Install the regular issues (default behavior)  -e, --essentials  Install the 'Essentials' collectionI am looking for ways to improve the user interface, the speed, and making it generally cleaner and more pythonic.Source code (also on GitHub):import urllib, os, re, time, sys, argparsefrom bs4 import BeautifulSoupISSUES_REGEX = u'MagPi[0-9]+'ESSENTIALS_REGEX = u'Essentials_.*'def get_installed(directory):    if not os.path.exists(directory):        if not quiet:            print(Directory doesn't exist\nCreating new directory with the name {}.format(directory))        os.mkdir(directory)    for filename in os.listdir(directory):        if re.match(regex, filename):            yield filenamedef download(filename, localfilename):    localfile = get_full_path(download_dir, localfilename)    try:        open(localfile, 'w').close()        urllib.request.urlretrieve(filename, localfile)    except KeyboardInterrupt:        print('Cleaning up...')        os.remove(localfile)        sys.exit()def webopen(page):    return urllib.request.urlopen(page)def get_links(soup):    for link in soup.find_all('a'):        yield link.get('href')def get_issues(soup):    links = list(get_links(soup))    for link in links:        if re.match(regex, link):            yield linkdef get_missing(installed, all_issues):    for issue in all_issues:        if not issue in installed:            yield issuedef get_full_path(directory, filename):    return os.path.join(directory, filename)def to_install_info(directory):    page = webopen(remote_dir)    soup = BeautifulSoup(page)    issues = list(get_issues(soup))    installed = list(get_installed(directory))    missing = list(get_missing(installed, issues))    return issues, installed, missingdef install(missing):    for issue in missing:        print('Downloading {} '.format(issue))        download(get_full_path(remote_dir, issue), issue)        print('Done')def install_quiet(missing):    for issue in missing:        download(get_full_path(remote_dir, issue), issue)def print_to_install_info(issues, installed, missing):    print('{} Released:\n\n{}\n\n{} Installed:\n\n{}\n\n{} To install:\n\n{}'.format(len(issues), issues, len(installed), installed, len(missing), missing))def install_all(missing):    install(missing)def install_all_quiet(missing):    install_quiet(missing)if __name__ == '__main__':    parser = argparse.ArgumentParser(description=Download issues of the MagPi magazine)    parser.add_argument('-q', '--quiet', action='store_true', help=Silence progress output)    parser.add_argument('-r', '--reinstall', action='store_true', help=Reinstall all issues)    parser.add_argument('--view', action='store_true', help=List the issues available for install)    parser.add_argument('-t',  dest='filetype', metavar='FILETYPE', type=str, action='store',            default=u'pdf', help=The extension of the files to download. Default: %(default)s)    parser.add_argument('-a', '--all', dest='types', action='store_const', const=u'.*',            help=Install all files with the right extension.)    parser.add_argument('-i', '--issues', dest='types', action='store_const', const=ISSUES_REGEX,            help='Install the regular issues (default behavior)')    parser.add_argument('-e', '--essentials', dest='types', action='store_const',            const=ESSENTIALS_REGEX, help=Install the 'Essentials' collection)    parser.add_argument('directory',  metavar='DIR', type=str,            help=The directory to install into.)    parser.add_argument('remote_dir',  metavar='REMOTE_DIR', type=str, nargs='?',            default='http://www.raspberrypi.org/magpi-issues/',            help=The directory to fetch from. Files must be links on that page. Works \                    best with Apache servers with the default directory listing. Default: %(default)s)    args = parser.parse_args()    download_dir = args.directory    remote_dir = args.remote_dir    types = args.types    if not types:        types = ISSUES_REGEX    regex = u^{}\.{}$.format(types, args.filetype)    print(regex)    quiet = args.quiet    reinstall = args.reinstall    view = args.view    issues, installed, missing = to_install_info(download_dir)    if reinstall:        missing = issues        msg = 'Overwriting issues'    else: msg = 'Installing Issues'    if not quiet:        print_to_install_info(issues, installed, missing)         if view:            print('To install, run without the view flag.')        else:            print(msg)            install_all(missing)    else:install_all_quiet(missing)I am sorry for the extreme lack of comments and docstrings, so that is what I will focus on.Usage cases:To download everything:$ ./magpi_downloader.py -a download_dirTo download all regular issues:$ ./magpi_downloader.py download_dirTo download the MagPi essentials:$ ./magpi_downloader.py -e download_dirTo view but not install available issues:$ ./magpi_downloader.py download_dirOther options are as documented above.
Magpi Magazine Downloader
python;python 3.x;web scraping
The code overall is not readable. It's quite lengthy, there is no modular structure, no meaningful comments and docstrings. The first thing I would do is to split it into multiple modules grouped logically, define the docstrings for each of the functions (which may actually lead to some functions being joined together or even removed).Also, some of the code blocks can be extracted into separate functions. For instance, apply the Extract Method refactoring method and extract the parsing arguments with argparse part into a separate function.Now, let's go over the issues one by one:import organization - avoid importing multiple built-in modules on a single line. Third-party imports need to have a newline before them. Put 2 newlines after all the imports (PEP8 import guidelines)since you are issuing multiple requests to the same domain, I would use requests module maintaining a web-scraping session via requests.Session - this might result into a significant performance boost:So if you're making several requests to the same host, the underlying TCP connection will be reused, which can result in a significant performance increasewhen instantiating a soup object, it's highly recommended to explicitly specify the underlying parser to avoid letting BeautifulSoup do this automatically - it might choose html.parser on your machine, but on the other machine, it may pick lxml or html5lib which may result into different parsing results (see Differences between parsers):soup = BeautifulSoup(page, html.parser)# soup = BeautifulSoup(page, html5lib)# soup = BeautifulSoup(page, lxml)you can improve the way you locate the elements on a page. Currently, you are finding all the links via find_all(a) and then applying a regular expression to the href attribute values. BeautifulSoup can do both in a single find command:soup.find_all(a, href=re.compile(rexpression here))the print_to_install_info() can benefit from using a multi-line string (to avoid multiple newline characters in the format template string)except for printing, the install() and install_quiet() functions really duplicate each other. Either have a single function with quiet argument, or, even better, use logging module with a configurable/controllable log levelsee if you can replace listdir() and an inner regex check with a single glob.glob() (or glob.iglob()) call.