the fox is black - a simple image scraper

Question

I wrote a simple image scraper for the Desktop Wallpaper project by The Fox is Black. The site is a fantastic (yet, apparently dead) blog full of fabulous images. However, there's no way to download an entire bundle of images, hence this simple scraper.

I'm putting the code up for scrutiny. Any areas for code style and/or implementation improvements? I'd appreciate your feedback.

#!/usr/bin/python

import os
import requests


from bs4 import BeautifulSoup as bs


MAX_PAGES = 1  # Max. number of pages is 41
SAVE_DIRECTORY = 'fox_backgrounds'
BASE_URL = "http://www.thefoxisblack.com/category/the-desktop-wallpaper-project/page"
RESOLUTIONS = [
            "1280x800", "1440x900", "1680x1050", "1920x1200", "2560x1440",
            "iphone", "iphone-5", "iphone6", "iphone-6-plus", "iphone6plus",
            "ipad",
            ]


def fetch_url(url):
    return requests.get(url).text


def clip_url(href):
    return href[href.rfind('/'):]    


def save_image(href):
    print(f"Downloading: {clip_url(href)}")
    request = requests.get(href)
    with open(f"fox_backgrounds{clip_url(href)}", 'wb') as output:
        output.write(request.content)


def get_images_from_page(url):
    html = fetch_url(url)
    soup = bs(html, "html.parser")
    for link in soup.find_all("a", class_="btn_download"):
        href = link["href"]
        for resolution in RESOLUTIONS:
            if resolution in href:
                save_image(href)


def make_dir():
    os.makedirs(SAVE_DIRECTORY, exist_ok=True)


def get_backgrounds():
    make_dir()
    for page in range(0, MAX_PAGES):
        get_images_from_page(BASE_URL + str(page + 1))


def main():
    get_backgrounds()


if __name__ == '__main__':
    main()

Answer 1

Suggested:

#!/usr/bin/python

import os
import requests


from bs4 import BeautifulSoup as bs
from pathlib import Path
from shutil import copyfileobj


MAX_PAGES = 1  # Max. number of pages is 41
SAVE_DIRECTORY = Path('fox_backgrounds')
BASE_URL = 'http://www.thefoxisblack.com/category/the-desktop-wallpaper-project/page'
RESOLUTIONS = {
    '1280x800', '1440x900', '1680x1050', '1920x1200', '2560x1440',
    'iphone', 'iphone-5', 'iphone6', 'iphone-6-plus', 'iphone6plus',
    'ipad'
}


def fetch_url(url):
    return requests.get(url).text


def clip_part(href):
    return href.rpartition('/')[-1]


def save_image(href):
    part = clip_part(href)
    print(f'   Downloading: {part}')
    fn = SAVE_DIRECTORY / part
    with requests.get(href, stream=True) as response, \
         open(fn, 'wb') as output:
        copyfileobj(response.raw, output)


def get_images_from_page(url):
    html = fetch_url(url)
    soup = bs(html, 'html.parser')
    for link in soup.find_all('a', class_='btn_download'):
        href = link['href']
        if any(href.endswith(f'-{res}.jpg') for res in RESOLUTIONS):
            save_image(href)
        else:
            print(f'Unknown resolution {href}')


def make_dir():
    os.makedirs(SAVE_DIRECTORY, exist_ok=True)


def get_backgrounds():
    make_dir()
    for page in range(1, MAX_PAGES+1):
        print(f'Fetching page {page}...')
        get_images_from_page(f'{BASE_URL}{page}')


def main():
    get_backgrounds()


if __name__ == '__main__':
    main()

Comments:

• You initialized SAVE_DIRECTORY but then use it for creation and not file write
• RESOLUTIONS should be a set, or maybe a tuple
• clip_url is slightly misleading; it returns a URL part and not a whole URL
• It's best if you have a prompt before downloading the index page; otherwise it hangs without the user knowing what's happening
• Stream your content so that you don't use up memory for big files
• Your RESOLUTIONS check is a little puzzling. Maybe it's a validation step? But if it's a validation step, you silently fail instead of printing a warning. Also, you keep iterating even after you've found the correct resolution. I rewrote this to just check the current resolution, and also be a little bit more careful about where it's seen in the filename.
• range(0, ...) is redundant, but for your use case you're better off with range(1 anyway.
• rpartition does basically the same thing as what you wrote, but doesn't require any fancy array slicing
• Don't call clip_path twice

---

Edit

The following version makes sane use of generators so that the iteration functions only need to know about their iteration, and not the inner business logic.

Also, your resolution check needs to be case-insensitive for many of the files on the site; and the site has gif and png images as well as jpg. You were missing some resolutions and some alternate iPhone spellings. I don't think that it's worth doing a resolution check at all, especially given these edge cases, but I left it in.

#!/usr/bin/python

import os
import requests


from bs4 import BeautifulSoup as bs
from pathlib import Path
from shutil import copyfileobj


MAX_PAGES = 1  # Max. number of pages is 41
SAVE_DIRECTORY = Path('fox_backgrounds')
BASE_URL = 'http://www.thefoxisblack.com/category/the-desktop-wallpaper-project/page'
RESOLUTIONS = {
    '1280x800', '1440x900', '1680x1050', '1920x1200', '2560x1440', '3840x2400',
    'iphone', 'iphone5', 'iphone-5', 'iphone6', 'iphone-6-plus', 'iphone6plus', 'iphone6-plus',
    'ipad'
}


def clip_part(href):
    return href.rpartition('/')[-1]


def save_image(href):
    part = clip_part(href)
    print(f'   Downloading: {part}')
    fn = SAVE_DIRECTORY / part
    with requests.get(href, stream=True) as response, \
         open(fn, 'wb') as output:
        copyfileobj(response.raw, output)


def urls_from_page(url):
    soup = bs(requests.get(url).text, 'html.parser')
    for link in soup.find_all('a', class_='btn_download'):
        href = link['href']
        if any(href.lower().contains(f'-{res}.') for res in RESOLUTIONS):
            yield href
        else:
            print(f'Unknown resolution {href}')


def make_dir():
    os.makedirs(SAVE_DIRECTORY, exist_ok=True)


def all_urls():
    for page in range(1, MAX_PAGES+1):
        print(f'Fetching page {page}...')
        yield from urls_from_page(f'{BASE_URL}{page}')


def main():
    make_dir()
    for url in all_urls():
        save_image(url)


if __name__ == '__main__':
    main()