3
\$\begingroup\$

I've written some code in python to scrape item names, category and the variety of images connected to each item from four different links out of a webpage. The main problem with which i had to hurdle is that each item may have one image or around five different images. Parsing those image links are no big deal but problem appears when it comes to save them in a local folder. Coder like me find it difficult cause few links contain single image and the rest contain several. It was hard for me to twitch the scraper in such a way so that all the images can be parsed and saved properly. I used four different links: two of which contain one image for each item and the rest contain five images for each item . However, my parser is running smoothly now. Here is what I did:

import requests
from lxml import html
import os

urls = [

    "http://www.floralart.com.au/products/39-melbourne-online-floristflorist-melbourne-flowers-delivery-funeral-flowers-free-online.aspx",
    "http://www.floralart.com.au/products/316-amore-mio-vase-arrangement.aspx",
    "http://www.floralart.com.au/products/90-wedding-florist-wedding-flowers-wedding-bouquet-bridal-flowers-flowers-for-weddings-artificial.aspx",
    "http://www.floralart.com.au/products/182-flowers-on-line-flower-bouquets-flower-deliveries-flowers-melbourne-cheap-flower-deliveries-mel.aspx",
]

def opening_link(link):

    response = requests.get(link)
    tree = html.fromstring(response.text)
    cat = tree.cssselect(".breadcrumb a")[1].text.strip()      #category name of the item

    for item in tree.cssselect(".product-details-page"):
        name = item.cssselect("h1.productname")[0].text.strip() if item.cssselect("h1.productname") else ""
        image_link = [img.attrib['src'] for img in item.cssselect(".picture a img")]             #for single image
        image_links = [img_link.attrib['src'] for img_link in item.cssselect(".overview a img") if '.jpeg' in img_link.attrib['src']]      #for list of images
        if image_links:           
            saving_images(cat,name,image_links)
        else:
            saving_images(cat,name,image_link)

def saving_images(cat, item_name, item_links):

    for link in item_links:
        response = requests.get(link, stream = True)
        image_name = link.split('/')[-1]

        if response.status_code == 200:
            os.chdir(r"C:\Users\ar\Desktop\mth")
            with open(image_name, 'wb') as f:
                for chunk in response.iter_content(1024):
                    f.write(chunk)

    print(item_name, cat)

if __name__ == '__main__':         
    for url in urls:
        opening_link(url)

Btw, if there are more than one image in a link then it is not necessary to make use of single image cause the list containing several images also contain the main image.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Code Style

There is a number of common PEP8 style guide violations which you may address:

  • imports to be properly organized
  • respect blank lines usage: remove the one in the urls list definition, use two newlines between the function definitions, remove extra blank lines at the beginning of the function bodies
  • inline comments should be 2 spaces from the right, there should be a single space after the # character
  • usage of spaces around the operators and in expressions and statements

Other Improvements

You can move some of your if checks from Python to CSS selectors. For instance:

[img_link.attrib['src'] for img_link in item.cssselect(".overview a img") if '.jpeg' in img_link.attrib['src']]

could be replaced with:

[img_link.attrib['src'] for img_link in item.cssselect(".overview a img[src$=jpeg]")]

When image_links is truthy, you send it to saving_images() function, which means that image_link is not used and you just wasted time defining it. How about refactoring it to be:

image_links = [img_link.attrib['src'] for img_link in item.cssselect(".overview a img[src$=jpeg]")]
if image_links:
    saving_images(cat, name, image_links)
else:  # single image
    image_link = [img.attrib['src'] for img in item.cssselect(".picture a img")]
    saving_images(cat, name, image_link)
\$\endgroup\$
1
  • \$\begingroup\$ It seems, the waiting time is over. Thanks sir a lot for your informative review. Always, new stuffs come up when you write anything. \$\endgroup\$
    – SIM
    Commented Oct 7, 2017 at 22:25
1
\$\begingroup\$

Removing mutability

urls = [
    ...
]

I would make this a tuple, so:

urls = (
    ...
)

This way you can't accidentally change it later (Well, you could mess up something in iteration...). If you attempt to, the interpreter will get upset at you and tell you exactly where the issue is instead of it silently failing and you having to spend a ridiculously long time to figure out the problem.

Naming

Functions are verbs that transform data ("nouns"). Instead of opening_link call it open_link. When you are inside the function you are opening the link, but what does the function do, it opens the link. (The same argument applies for saving_images. Call it save_images).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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