5
\$\begingroup\$

This is a work in progress, as you may read I plan to revise to utilise JSON data rather than directly scraping.

I am considering refactoring the try//except statements somewhat so that each set encapsulates only one possible fail/error point and handles that, rather than multiple except statements.

I'm also very open to feedback on code-style and documentation.

#! python3
# downloadXkcd.py - Downloads every single XKCD comic.
"""
Webscraper that downloads xkcd comics.
Checks if comic already downloaded so for increased efficiency on rerun.

Two run modess: Full and Quick
Full mode goes through every comic.
Quick mode quits when it reaches the first comic that is already downloaded.

Feature updates - multithreading, max 100 comics/thread.
To implement dual modes required iterating backwards through the comics in
each thread until already downloaded comic found.

Planned feature update where title text is in properties of downloaded image.

Planned feature update implement counts to provide feedback as to how many
comics downloaded in current run. 

Planned change to use JSON data, rather than downloading page for each comic.


Derived from original project: https://automatetheboringstuff.com/chapter11/

@author: david.antonini // toonarmycaptain
"""
import time
import os
import requests
import bs4
import threading

print('This script searches xkcd.com and downloads each comic.')

# User input for full run or until finding already downloaded comic.
print('There are two mode options:\n'
      '\nQuick mode: Or "refresh mode", checked until it finds '
      'a previously downloaded comic.\n'
      ' Full mode: Checks for every comic, downloads undownloaded comics.\n'
      )

while True:
    try:
        print('Please select mode:\n'
              'Enter 0 for Quick mode, or 1 for Full Mode')
        run_mode_selection = input('Mode: ')
        if int(run_mode_selection) == 0:
            run_mode = False  # Quick mode
            break
        if int(run_mode_selection) == 1:
            run_mode = True    # Full mode
            break
    except ValueError:
        continue

start = time.time()


os.makedirs('xkcd', exist_ok=True)   # store comics in ./xkcd


def download_xkcd(startComic, endComic, direction):
    """
    Iterate over comic numbers, download comic page, find comic image, check if
    file with comic name already exists, if not, download comic image.

    Args:
        startComic (int): the number of the first comic thread iterates over.
        endComic (int): the number of the last comic thread iterates over.
        direction (int): 1 or -1 iterating forwards or backwards based on mode.
    """
    for urlNumber in range(startComic, endComic, direction):
        # Download the page.
#        print(f'Downloading page http://xkcd.com/{urlNumber}...')
        try:
            res = requests.get(f'http://xkcd.com/{urlNumber}')
            res.raise_for_status()
            soup = bs4.BeautifulSoup(res.text, 'lxml')
        except requests.exceptions.HTTPError:
            continue
        # Find the URL of the comic image.
        comicElem = soup.select('#comic img')
        if comicElem == []:
            print(f'Could not find comic image {urlNumber}.')
        else:
            try:
                comicUrl = 'https:' + comicElem[0].get('src')
                # Download the image.
                res = requests.get(comicUrl)
                res.raise_for_status()
                # Check if comic previously downloaded.
                imageFile = open(os.path.join(
                        'xkcd',
                        (f'{urlNumber} - {os.path.basename(comicUrl)}')), 'xb')
                print(f'Downloading image {comicUrl}...')

                # Save the image to ./xkcd
                for chunk in res.iter_content(100000):
                    imageFile.write(chunk)
                imageFile.close()

            # TODO: Needs feature update where title text
            #       is in properties of downloaded image.

            except requests.exceptions.MissingSchema:
                print(f'--- Missing comic {urlNumber}.---')
                continue  # skip this comic
            except FileExistsError:
#                print(f'--- Comic {urlNumber} already downloaded.---')
                if run_mode:   # Full mode
                    continue  # skip this comic
                if not run_mode:
#                    print(f'Finished updating archive, '
#                          f'comics {startComic}-{endComic}.')
                    break


# Get latest comic number:
url = 'https://xkcd.com'
res = requests.get(url)
res.raise_for_status()
soup = bs4.BeautifulSoup(res.text, 'lxml')
penultimateComic = soup.select('a[rel="prev"]')[0]
# penultimate Comic +1 for most recent comic
finalComicNum = int(penultimateComic.get('href')[1:-1]) + 1


# Create and start the Thread objects.
downloadThreads = []  # a list of all the Thread objects
for i in range(0, finalComicNum, 100):
    if run_mode:
        downloadThread = threading.Thread(target=download_xkcd,
                                          args=(i, i+100, 1))
    if not run_mode:  # quick mode iterates back until pre-existing file
        downloadThread = threading.Thread(target=download_xkcd,
                                          args=(i+100, i, -1))
    downloadThreads.append(downloadThread)
    downloadThread.start()

# Wait for all threads to end.
for downloadThread in downloadThreads:
    downloadThread.join()

print('Done.')

timetotal = time.time() - start
if timetotal > 60:
    mins = timetotal//60
    sec = timetotal-mins*60
    print(f"Runtime: {mins:.0f} minutes, {sec:.2f} seconds")
else:
    print(f"Runtime: {timetotal:.2f} seconds")
\$\endgroup\$
6
\$\begingroup\$

First of all, you have multiple PEP8 code style violations, like using camel case styled variable naming instead of a "lower case with underscores"; organizing, grouping imports, comments not formatted properly.

I would also apply the following improvements:

  • use requests.Session() to re-use a single web-scraping session to make requests - this should have a positive impact on performance
  • apply "Extract Method" refactoring method for the "download image" code block
  • use select_one() to locate a single element by a CSS selector

The relevant part of the program with some of the changes applied:

def download_image(session, url, filename):
    res = session.get(url)
    res.raise_for_status()

    with open(os.path.join('xkcd', filename), 'xb') as image_file:
        print(f'Downloading image {comic_url}...')

        for chunk in res.iter_content(100000):
            image_file.write(chunk)


def download_xkcd(comic_start, comic_end, direction):
    """
    Iterate over comic numbers, download comic page, find comic image, check if
    file with comic name already exists, if not, download comic image.

    Args:
        comic_start (int): the number of the first comic thread iterates over.
        comic_end (int): the number of the last comic thread iterates over.
        direction (int): 1 or -1 iterating forwards or backwards based on mode.
    """
    with requests.Session() as session:
        for url_number in range(comic_start, comic_end, direction):
            try:
                res = session.get(f'http://xkcd.com/{url_number}')
                res.raise_for_status()
                soup = bs4.BeautifulSoup(res.text, 'lxml')
            except requests.exceptions.HTTPError:
                continue

            comic_image = soup.select_one('#comic img[src]')
            if not comic_image:
                print(f'Could not find comic image {url_number}.')
                return

            try:
                comic_url = 'https:' + comic_image['src']
                download_image(session, comic_url, f'{url_number} - {os.path.basename(comic_url)}')
            except requests.exceptions.MissingSchema:
                print(f'--- Missing comic {url_number}.---')
                continue  # skip this comic
            except FileExistsError:
                if run_mode:   # Full mode
                    continue  # skip this comic
                if not run_mode:
                    break
\$\endgroup\$
  • \$\begingroup\$ Thankyou! I had not gone as far into Beautiful Soup as to encounter requests.Session() or select_one() yet. \$\endgroup\$ – toonarmycaptain Oct 19 '17 at 14:06
  • \$\begingroup\$ What is the advantage of extracting the download code from the rest, is it mostly readability? \$\endgroup\$ – toonarmycaptain Oct 19 '17 at 14:07
  • \$\begingroup\$ Can you explain (aside from the commented out print statements) what was wrong with my comment formatting? \$\endgroup\$ – toonarmycaptain Oct 19 '17 at 14:22
  • 1
    \$\begingroup\$ @toonarmycaptain yup, 1) readability and modularity. 2) this is a very minor note, but according to PEP8 you should format comments this way. 3) right, define imports in groups (first system-level, then third-party, then your local-level) which are also sorted alphabetically. Also, I usually import from bs4 import BeautifulSoup, but that's just probably my preference. Thanks. \$\endgroup\$ – alecxe Oct 19 '17 at 15:31
  • 1
    \$\begingroup\$ @toonarmycaptain I've actually got a better idea - to use a with context manager - it's a recommended way to go when dealing with files. Updated the answer accordingly. \$\endgroup\$ – alecxe Oct 19 '17 at 17:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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