7
\$\begingroup\$

Here is a pure Python script that saves an HTML page without CSS but with all images on it and replaces all hrefs with a path of an image on the hard drive.

I know that there are great libraries like BeautifulSoup and others but I would like to try myself with pure Python.

Actually there is no practical usage of this script, it was just test task from one of companies where I applied.

Once again what it does:

This script can pre-launched from command line, which takes 2 arguments:

  1. Address of web page to save (this one is required)
  2. Name of folder where images from page should be saved (this one is optional) Script saves html content of page(without CSS) and also it looks for all images on page and save them too, replacing their href attribute to actual path of image on hard drive.

How can I improve it?

import random
import string
import sys
import urllib2
import os
import re
from urlparse import urlparse

def page_loader(url_name, dir_name='imgs'):

    page_to_open = urllib2.urlopen(url_name)
    target_page = page_to_open.read()
    base_dir = os.path.dirname(os.path.realpath(__file__))
    dir_to_save = os.path.join(base_dir, dir_name)
    new_file_name = '%s.html' % ''.join(random.choice(string.ascii_uppercase + string.ascii_lowercase) for _ in range(10))
    if not os.path.exists(dir_to_save):
        os.makedirs(dir_to_save)


    images_on_page = re.findall('img .*?src="(.*?)"', target_page)
    internal_images = [img for img in images_on_page if img.startswith('/')]
    external_images = [img for img in images_on_page if not img.startswith('/')]

    for image in internal_images:
        image_url = '%s%s' % (page_to_open.geturl()[:-1], image)
        new_image_name = urlparse(image_url).path.split('/')[-1]
        with open(os.path.join(dir_to_save, new_image_name), 'w') as new_image:
            new_image.write(urllib2.urlopen(image_url).read())
            target_page = re.sub(image, new_image.name, target_page)

    for image_url in external_images:
        new_image_name = urlparse(image_url).path.split('/')[-1]
        with open(os.path.join(dir_to_save, new_image_name), 'w') as new_image:
            new_image.write(urllib2.urlopen(image_url).read())
            target_page = re.sub(image_url, new_image.name, target_page)

    with open(os.path.join(base_dir, new_file_name), 'w') as new_file:
        new_file.write(target_page)

if __name__ == '__main__':
    target_url = sys.argv[1]
    if len(sys.argv) > 2:
        dir_name = sys.argv[2]
        page_loader(target_url, dir_name)
    else:
        page_loader(target_url)
\$\endgroup\$
6
\$\begingroup\$

Your code is pretty good! I do have a few tips regarding style, and such.

  • As mentioned in the comments, even though it looks like you aren't, you shouldn't be trying to parse HTML with Regexes.
  • Where are your comments? While good code can be pretty readable, comments are still a valuable asset. You should probably flesh out your page_loader function with a docstring, and any unclear blocks of code with an inline comment.
  • If you're only going to use part of a module, e.g, one function or variable, you should do from ... import ....

That's about all I can really think of right now. If there's anything else you want me to comment on, just mention it below, and I'll see if I can cover it. Hope this helps!

\$\endgroup\$
3
\$\begingroup\$

You have some very long lines. Making them shorter will improve readability on smaller displays and when your IDE has right and left panes open.

EDIT: This is the line I was talking about (sorry for saying multiple lines)

new_file_name = '%s.html' % ''.join(random.choice(string.ascii_uppercase + string.ascii_lowercase) for _ in range(10))

I usually break the lines on the + sign, whenever possible. This is how I'd do it:

new_file_name = '%s.html' % ''.join(random.choice(string.ascii_uppercase +
                                    string.ascii_lowercase) for _ in range(10))

This is one of the recommendations from Python's style guide, which I prefer, and it's applicable when you have parenthesis. If you don't, you're advised to put some. You can also use the backslash to separate the lines, like this:

new_file_name = '%s.html' % ''.join(random.choice(string.ascii_uppercase + \
    string.ascii_lowercase) for _ in range(10))

The key point is that, whatever means you choose, you need to clearly separate the continuation's indentation from the indentations of what's above and below.

\$\endgroup\$

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.