Image downloader for a website

Question

This code takes a website and downloads all .jpg images in the webpage. It supports only websites that have the <img> element and src contains a .jpg link.

(Tested here)

import random
import urllib.request
import requests
from bs4 import BeautifulSoup

def Download_Image_from_Web(url):
    source_code = requests.get(url)
    plain_text = source_code.text
    soup = BeautifulSoup(plain_text, "html.parser")
    raw_text = r'links.txt'
    with open(raw_text, 'w') as fw:
        for link in soup.findAll('img'):
            image_links = link.get('src')
            if '.jpg' in image_links:
                for i in image_links.split("\\n"):
                    fw.write(i + '\n')
    num_lines = sum(1 for line in open('links.txt'))
    if num_lines == 0:
        print("There is 0 photo in this web page.")
    elif num_lines == 1:
        print("There is", num_lines, "photo in this web page:")
    else:
        print("There are", num_lines, "photos in this web page:")
    k = 0
    while k <= (num_lines-1):
        name = random.randrange(1, 1000)
        fullName = str(name) + ".jpg"
        with open('links.txt', 'r') as f:
            lines = f.readlines()[k]
            urllib.request.urlretrieve(lines, fullName)
            print(lines+fullName+'\n')
        k += 1

Download_Image_from_Web("https://pixabay.com")

Answer 1

Unnecessary file operations

This is horribly inefficient:

k = 0
while k <= (num_lines-1):
    name = random.randrange(1, 1000)
    fullName = str(name) + ".jpg"
    with open('links.txt', 'r') as f:
        lines = f.readlines()[k]
        urllib.request.urlretrieve(lines, fullName)
        print(lines+fullName+'\n')
    k += 1

Re-reading the same file num_lines times, to download the k-th!

Btw, do you really need to write the list of urls to a file? Why not just keep them in a list? Even if you want the urls in a file, you could keep them in a list in memory and never read that file, only write.

Code organization

Instead of having all the code in a single function that does multiple things, it would be better to organize your program into smaller functions, each with a single responsibility.

Python conventions

Python has a well-defined set of coding conventions in PEP8, many of which are violated here. I suggest to read through that document, and follow as much as possible.

Answer 2

Aside from the things others mentioned, you can also improve the way you locate the img elements that have src attribute ending with .jpg. Instead of using findAll and if conditions, you can do it in one go with a CSS selector:

for img in soup.select("img[src$=jpg]"):
    print(img["src"])

Answer 3

How about the following?

import random
import requests
from bs4 import BeautifulSoup

# got from http://stackoverflow.com/a/16696317
def download_file(url):
    local_filename = url.split('/')[-1]
    print("Downloading {} ---> {}".format(url, local_filename))
    # NOTE the stream=True parameter
    r = requests.get(url, stream=True)
    with open(local_filename, 'wb') as f:
        for chunk in r.iter_content(chunk_size=1024): 
            if chunk: # filter out keep-alive new chunks
                f.write(chunk)
    return local_filename

def Download_Image_from_Web(url):
    source_code = requests.get(url)
    plain_text = source_code.text
    soup = BeautifulSoup(plain_text, "html.parser")
    for link in soup.findAll('img'):
        image_links = link.get('src')
        if not image_links.startswith('http'):
            image_links = url + '/' + image_links
        download_file(image_links)

Download_Image_from_Web("https://pixabay.com")