Downloading images from URLs

Question

I'm just beginning with Python and programming, so been trying to get as much experience reading code as possible.

The script mentioned below do grab images from URLs and put them in into a folder. The code is tested and works fine. Is there a way to shorten/improve this code? Any ideas/suggestions are appreciated.

import urllib2
from os.path import basename
from urlparse import urlsplit
from bs4 import BeautifulSoup # for HTML parsing

global urlList
urlList = []

# recursively download images starting from the root URL
def downloadImages(url, level): # the root URL is level 0
    print url
    global urlList
    if url in urlList: # prevent using the same URL again
        return
    urlList.append(url)
    try:
        urlContent = urllib2.urlopen(url).read()
    except:
        return

    soup = BeautifulSoup(''.join(urlContent))
    # find and download all images
    imgTags = soup.findAll('img')
    for imgTag in imgTags:
        imgUrl = imgTag['src']
        try:
            imgData = urllib2.urlopen(imgUrl).read()
            fileName = basename(urlsplit(imgUrl)[2])
            output = open(fileName,'wb')
            output.write(imgData)
            output.close()
        except:
            pass

    # if there are links on the webpage then recursively repeat
    if level > 0:
        linkTags = soup.findAll('a')
        if len(linkTags) > 0:
            for linkTag in linkTags:
                try:
                    linkUrl = linkTag['href']
                    downloadImages(linkUrl, level - 1)
                except:
                    pass

# main
downloadImages('http://www.yahoo.com', 1)

Python version 2.6.

Answer 1

Notes about your code

• You can also simplify the way you look for the links to follow:

  for link in soup.select("a[href]"):
      downloadImages(link["href"], level - 1)
  

  Here, we are enforcing the a elements to have href values, not checking for elements to be found (since the loop body would just not be executed in this case).

• As far as managing the urls you've already visited:

  • don't use globals
  • use a set instead of a list for faster lookups

• It's also a good practice to always specify the parser BeautifulSoup uses under-the-hood:

  soup = BeautifulSoup(urlContent, "lxml")  
  # or soup = BeautifulSoup(urlContent, "html.parser")
  # or soup = BeautifulSoup(urlContent, "html5lib")
  

• follow PEP8 recommendations - specifically, naming is agreed to be in lower_case_with_underscores format, not camelCase

• no need to join the urlContent - simply pass urlContent to BeautifulSoup

Alternative solution

I would implement it this way:

try:
    from urlparse import urljoin
except ImportError:
    from urllib.parse import urljoin

import requests
from bs4 import BeautifulSoup


class Scraper:
    def __init__(self):
        self.visited = set()
        self.session = requests.Session()
        self.session.headers = {"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.109 Safari/537.36"}

        requests.packages.urllib3.disable_warnings()  # turn off SSL warnings

    def visit_url(self, url, level):
        print(url)
        if url in self.visited:
            return

        self.visited.add(url)

        content = self.session.get(url, verify=False).content
        soup = BeautifulSoup(content, "lxml")

        for img in soup.select("img[src]"):
            image_url = img["src"]
            if not image_url.startswith(("data:image", "javascript")):
                self.download_image(urljoin(url, image_url))

        if level > 0:
            for link in soup.select("a[href]"):
                self.visit_url(urljoin(url, link["href"]), level - 1)

    def download_image(self, image_url):
        local_filename = image_url.split('/')[-1].split("?")[0]

        r = self.session.get(image_url, stream=True, verify=False)
        with open(local_filename, 'wb') as f:
            for chunk in r.iter_content(chunk_size=1024):
                f.write(chunk)


if __name__ == '__main__':
    scraper = Scraper()
    scraper.visit_url('http://www.yahoo.com', 1)

Aside from things mentioned above, here are some other applied changes:

• using requests third-party library with a shared session
• Python 2 and 3 compatible
• using class for sharing "session" and a set of visited urls
• separate methods to visit urls and download images
• if __name__ == "__main__": is used to avoid the code being executed on import
• changed the way image filename is determined (probably still not the best way)
• handling relative urls as well as absolute

Answer 2

Catching exceptions

It's generally not a good idea to use try-except blocks when you aren't specifying what Exceptions you want to catch, as you've done here:

try:
    urlContent = urllib2.urlopen(url).read()
except:
    return

As well as here:

try:
    imgData = urllib2.urlopen(imgUrl).read()
    fileName = basename(urlsplit(imgUrl)[2])
    output = open(fileName,'wb')
    output.write(imgData)
    output.close()
except:
    pass

You should specify exactly which Exceptions you are catching. If you don't, then every single Exception raised under the try block will be ignored, including ones caused by errors in your code. Ideally, you should also output to the user what exactly went wrong. Here's an example:

try:
    some_function(argument1, argument2)
except (ExceptionA, ExceptionB) as exception:
    print "X thing failed due to Y. Using Z instead may solve this"

Opening files

In your code you open files using the standard open and close methods provided, like so:

output = open(fileName,'wb')
output.write(imgData)
output.close()

While there's not much that's inherently wrong with doing this, it's almost always better (and safer) to use a context manager to open files. This means that the above code, using a context manager, would become this:

with open(fileName, 'wb') as output:
    output.write(imgData)

Nitpicks

Other than the two things listed above, I really only have two other criticisms of your code, those two criticisms being purely stylistic in nature:

1. function names should be in snake_case, not camelCase
2. variable names should also be in snake_case and not camelCase

These two naming criticisms are documented in PEP8, the official style guide for Python. I'd recommend taking a look at it.

Answer 3

The use of global is generally discouraged and isn't needed in this example. You can leave out global and the code will function just fine.