Scrapy spider for products on a site

I recently submitted a code sample for a web scraping project and was rejected without feedback as to what they didn't like. The prompt, while I cannot give it here verbatim, basically stated that I needed to write a spider to crawl a site for product items. They suggested using a generic spider to scrape the site in question while using URL rules for efficiency. They gave links to documentation in case you hadn't used scrapy before. I felt like this meant that they didn't mind hiring people unfamiliar with their toolset.

Speaking of which we could only use pyquery for dom traversal. I usually would have opted for pure lxml and xpaths.

I understood the concept of using rules to limit extraneous requests but after noticing that the site in question contained a sitemap I decided to start there instead.

I do know that they explicitly said not to use any outside libraries, so that is why I didn't use Pillow for image processing. However, I did cheat and use requests for some other things that the actual spider didn't utilize but again I wasn't told why my code wasn't good enough. So at this point I would like to learn why.

# -*- coding: utf-8 -*-
import scrapy
from scrapy.spiders.sitemap import *
from pyquery import PyQuery as pq
from oxygendemo.items import OxygendemoItem
import oxygendemo.utilities
from oxygendemo.utilities import *

class OxygenSpider(SitemapSpider):

print 'MY SPIDER, IS ALIVE'
name = "oxygen"
allowed_domains = ["oxygenboutique.com"]
sitemap_urls = ['http://www.oxygenboutique.com/sitemap.xml']
sitemap_rules = generate_sitemap_rules()
ex_rates = get_exchange_rates()

def parse_sitemap_url(self, response):

self.logger.info('Entered into parse_sitemap_url method')
self.logger.debug('Respons status: {}'.format(response.status))

item = OxygendemoItem()
d = pq(response.body)
parsed_url = urlparse.urlparse(response.url)
base_url = get_base(parsed_url)
product_info = d('.right div#accordion').children()
image_links = d('div#product-images tr td a img')
description = product_info.eq(1).text()\
.encode('ascii', 'ignore')
item['code'] = str(parsed_url[2].lstrip('/')[:-5])
item['description'] = description
item['name'] = d('.right h2').text()

gbp_price = {
'prices': d('.price').children(),
'discount': 0
}

item['gbp_price'], item['sale_discount'] = get_price_and_discount(
gbp_price
)

if 'error' not in self.ex_rates:

item['usd_price'] = "{0:.2f}".format(
item['gbp_price'] * self.ex_rates['USD']
)
item['eur_price'] = "{0:.2f}".format(
item['gbp_price'] * self.ex_rates['EUR']
)
else:

item['usd_price'], item['eur_price'] = ['N/A'] * 2

item['designer'] = d('.right').find('.brand_name a').text()
item['stock_status'] = json.dumps(determine_stock_status(d('select')
.children()))
item['gender'] = 'F'  # Oxygen boutique carries Womens's clothing
item['raw_color'] = get_product_color_from_description(description)

yield item


This is the utilities module I used:

# -*- coding: utf-8 -*-
import requests
import json
import urlparse
from pyquery import PyQuery as pq
import re

def get_base(parsed_url):

base_url = parsed_url[0] + '://' + parsed_url[1]
base_url = base_url.encode('ascii', 'ignore')
return base_url

def get_exchange_rates():
''' return dictionary of exchange rates with british pound as base
currency '''

url = 'http://api.fixer.io/latest?base=GBP'
try:
response = requests.get(url)
return er
except:
return {'error': 'Could not contact server'}

def determine_stock_status(sizes):

result = {}
for i in xrange(1, len(sizes)):
option = sizes.eq(i).text()
if 'Sold Out' not in option:
result[option] = 'In Stock'
else:
size = option.split(' ')[0]
result[size] = 'Sold Out'
return result

def determine_type(short_summary):

short_summary = short_summary.upper()

S = {
'HEEL', 'SNEAKER', 'SNEAKERS',
'BOOT', 'FLATS', 'WEDGES',
'SANDALS'
}

J = {
'RING', 'NECKLACE', 'RING',
'BANGLE', 'CHOKER', 'COLLIER',
'BRACELET', 'TATTOO', 'EAR JACKET'
}

B = {
'BAG', 'PURSE', 'CLUTCH',
'TOTE'
}

A = {
'PINNI', 'BLOUSE', 'TOP',
'SKIRT', 'KNICKER', 'DRESS',
'DENIM', 'COAT', 'JACKET',
'SWEATER', 'JUMPER', 'SHIRT',
'SKINNY', 'SHORT', 'TEE',
'PANTS', 'JUMPSUIT', 'HIGH NECK',
'GOWN', 'TROUSER', 'ROBE',
'PLAYSUIT', 'CULOTTE', 'JODPHUR',
'PANTALON', 'FLARE', 'CARDIGAN',
'VEST', 'CAMI', 'BEDSHORT',
'PYJAMA', 'BRALET', 'TUNIC',
'HOODY', 'SATEEN', 'BIKER',
'JEAN', 'SWEAT', 'PULL',
'BIKINI', 'LE GRAND GARCON'
}

types = {
'B': B, 'S': S,
'J': J, 'A': A
}

for key, val in types.iteritems():
for t in val:
if t in short_summary:
return key
else:
return 'R'  # Tag as accessory as failsafe

''' base_url will come as unicode change to python string '''

images = []

images.append(urlparse.urljoin(base_url, image.attrib['src']))

return images

def get_price_and_discount(gbp_price):

if gbp_price['prices']('.mark').text() == '':  # No discount
gbp_price['discount'] = '0%'
orig_price = float(gbp_price['prices'].parent().text()
.encode('ascii', 'ignore'))
else:  # Calculate discount
prices = gbp_price['prices']
orig_price = "{0:.2f}".format(float(prices('.mark').text()))
new_price = "{0:.2f}".format(float(gbp_price['prices'].eq(1).text()))
gbp_price['discount'] = "{0:.2f}"\
.format(float(orig_price) / float(new_price) * 100) + '%'
return float(orig_price), gbp_price['discount']

def get_raw_image_color(image):
''' Note that Pillow imaging library would be perfect
for this task. But external libraries are not
allowed via the constraints noted in the instructions.
Example: Image.get_color(image)
Could be used with Pillow.
'''

# only import Pillow image library if this is used
# Later
from PIL import Image
im = Image.open(image)
colors = im.getcolors()
if colors is None:
return None
else:
return colors[0]  # Not functional at this point

def get_product_color_from_description(description):
''' Will go this route to avoid external imports '''

description = description.upper().split(' ')
colors = (
'BLACK', 'WHITE', 'BLUE',
'YELLOW', 'ORANGE', 'GREY',
'PINK', 'FUSCIA', 'RED',
'GREEN', 'PURPLE', 'INDIGO',
'VIOLET'
)

for word in description:
for color in colors:
if word == color:
return color.lower()
else:
return None

def generate_sitemap_rules():

d = pq(requests.get('http://www.oxygenboutique.com').content)

# Proof of concept regex can be found here --> http://regexr.com/3c0lc
designers = d('ul.tame').children()
re_front = r'(http:\/\/)(www\.)(.+\/)((?!'
re_back = r').+)'
re_middle = 'products|newin|product|lingerie|clothing'

for li in designers:
''' This removes 36 requests from the queue '''

return [(re_front + re_middle.replace('-', r'\-') + re_back,
'parse_sitemap_url')]


OxygendemItem() declaration:

import scrapy
from scrapy import Field

class OxygendemoItem(scrapy.Item):

code = Field()  # unique identifier (retailers perspective)
description = Field()  # Detailed description
designer = Field()  # manufacturer
eur_price = Field()  # full (non_discounted) price
gender = Field()  # F - Female, M - male
gbp_price = Field()  # full (non_discounted) price
image_urls = Field()  # list of urls representing the item
link = Field()  # url of product page
name = Field()  # short summary of the item
raw_color = Field()  # best guess of color. Default = None
sale_discount = Field()  # % discount for sale item where applicable
stock_status = Field()  # dictionary of sizes to stock status
'''
size: quantity
Example: { 'L': 'In Stock',
'M': 'In Stock',
'S': 'In Stock',
'XS': 'In Stock'
}
'''
# 'A' = apparel, 'B' = bags, 'S' = shoes, 'J' = jewelry, 'R' = accessories
type = Field()
usd_price = Field()  # full (non_discounted) price


Well to start with you have bad practices in your imports. It's recommended to stay away from using from module import * because doing that imports things without explicitly declaring their names. Without realising it, you could be overwriting other functions, including builtins in the module was made carelessly. Instead use just import module or from module import func1, func2, CONST. Especially though, don't do this:

import oxygendemo.utilities
from oxygendemo.utilities import *


It's totally redundant to have the first line since you're then ignoring it to import everything. In case you don't know, you can still alias plain imports:

import oxygendemo.utilities as util


So you don't even need to worry about the name being too long.

Also OxygenSpider is not laid out properly. You have loose code that should probably be in an __init__ function. Let me show you how this works in the interpreter:

>>> class A:
print "Printing class A"

Printing class A


So what happened there? The print command was run when the class was created. I haven't created any object yet, so what happens when I create an object:

>>> A()
<__main__.A instance at 0x0000000002CA5588>
>>> b = A()
>>>


Nothing. It's not printing the command that you intended to appear when creating an OxygenSpider object. If you were to wrap it in __init__ though, it would. __init__ is a special function that runs when a new object is created, like so:

>>> class A:
def __init__(self):
print "Printing this object"

>>> A()
Printing this object
<__main__.A instance at 0x0000000002113488>
>>> b = A()
Printing this object


You see now? Nothing happens after the class is created but when actual objects are created __init__ gets run. You should be putting the whole opening block to OxygenSpider in a function like that. Also the variables should be assigned as self.var, and the constants should be in UPPER_SNAKE_CASE and constant lists should be tuples instead. Tuples are made with () and are basically like lists except they cannot be changed.

However since you're inheriting from SitemapSpider you also need to run its __init__ function in yours. You need to call it so that your base class is initialised before you run your particular __init__ code. There's a good explanation in this Stack Overflow answer

class OxygenSpider(SitemapSpider):

def __init__(self):
super(SitemapSpider, self).__init__()
print 'MY SPIDER, IS ALIVE'
self.NAME = "oxygen"
self.ALLOWED_DOMAINS = ("oxygenboutique.com")
self.SITEMAP_URLS = ('http://www.oxygenboutique.com/sitemap.xml')
self.sitemap_rules = generate_sitemap_rules()
self.ex_rates = get_exchange_rates()


Also printing when creating an object just to say it's created isn't very nice anyway, you should remove that.

• Ahh ok thanks for that @SuperBiasedMan. I tried doing declaring instance variables with self initially but then I ran into self not defined error. But I guess that wouldn't have happened had I used __init__() properly. – trendsetter37 Oct 23 '15 at 13:56
• @trendsetter37 Yeah, for self to work it needs to be in the function, since __init__ takes self as the parameter you can refer to. – SuperBiasedMan Oct 23 '15 at 14:02
• placing the name attribute within the init method yields a spider not found error. In the docs they declare name outside of init in their examples. Is that absolutely necessary or does their codebase require that for everything to work? – trendsetter37 Oct 23 '15 at 14:54
• @trendsetter37 This is because you're now overriding their __init__ method which declare a spider and launch all the logic. Even though it is good practice to use an __init__ to initialize things, in this case it is just not what you should do. – 301_Moved_Permanently Oct 23 '15 at 15:39
• Argh. Apologies, I forgot about the inheritance. I added a note about calling __init__. It should work fine with the added line that calls on SitemapSpider's __init__. – SuperBiasedMan Oct 23 '15 at 16:09

Your utilities module is too specific

Separating the code into data model, utilities functions and proper logic is generally a good idea. But in your code, the utilities module contains functions that are tightly coupled with your spider. I’d rather use them as @staticmethods of your spider.

Except maybe for get_exchange_rates and get_base.

However, for such an exercise, I’d put them all into a single file since it’s easier to handle and reduce import cluttering.

Stay consistent

Whatever coding style you choose to apply, stay consistent. For instance, in your items.py file, you define class OxygendemoItem(scrapy.Item): and then your define your fields using Field(). Either use scrapy.Item and scrapy.Field() or Item and Field() but do not mix them. Same for the indentation level of closing parenthesis or braces…

Use higher-level (“more pythonic”) constructs

Some looping constructs would benefit from using the right data structure to simplify the logic.

Compare your fetch_images with:

[urlparse.urljoin(base_url, image.attrib['src']) for image in image_links]


no need to define a function anymore. Same for your get_product_color_from_description:

description = set(description.upper().split())
colors = {
'BLACK', 'WHITE', 'BLUE',
'YELLOW', 'ORANGE', 'GREY',
'PINK', 'FUSCIA', 'RED',
'GREEN', 'PURPLE', 'INDIGO',
'VIOLET'
}
try:
return (description & colors).pop()
except KeyError:
return None


By the way, for this one:

• colors should be a global variable; same for other collections you build elsewhere in your utilities module;
• use split without parameters, it will remove multiple blanks (spaces, tabs, newlines) before splitting the string;
• remember to always put an exception type in the except clause (unlike what you did with get_exchange_rates).

Use what you are told to use

That might be the root cause of them not even giving feedback to you. For whatever reason they have their environment set-up so that a small amount of libraries are available, if you provide them code that requires external modules it won't run. Why would they take time to answer someone that didn't take time to test its code before submitting it?

The worse part is get_raw_image_color. You're not even calling it. If you truly want to explain that you know how to use an external tool that would better fit for the task, put it as a comment of your submission. But in the code it has no added value, it's just noise.

[Speculative] Use scrapy builtin capabilities

As far as I understand (I'm not a scrapy user), scrapy installation requires lxml and provides DOM manipulation through xpath and css selectors. If you are more used to them, go for it.

I don't know if scrapy can be installed without lxml, though, so I might stick with PyQuery just in case.

Examine the problem

Even though you were told to use url rules, they are not really of added value here. Looking at the sitemap, every url is of the form http://www.oxygenboutique.com/<page_name>.aspx with little to no indication of whether a given page is about a product or something else. Filtering on categories names as you do might even filter out some actual products. Using a SitemapSpider does a pretty good filtering by itself that you should only need to check if the page is actually a product in your parsing function.

Miscellaneous

• What's with that yield item?
• Why a dict for the stock statuses instead of a list of available options?
• Why a gender field if it's going to be constant?
• What's with these empty line right after : (defs, ifs…)?
• What's with all these float to str to float conversions to get prices and discounts?

Draft for improvement

# -*- coding: utf-8 -*-
import scrapy
import json
from pyquery import PyQuery
import urlparse
import urllib2

PRODUCTS_TYPES = {
'S': {
'HEEL', 'SNEAKER', 'SNEAKERS',
'BOOT', 'FLATS', 'WEDGES',
'SANDALS'
},
'J': {
'RING', 'NECKLACE', 'RING',
'BANGLE', 'CHOKER', 'COLLIER',
'BRACELET', 'TATTOO', 'EAR JACKET'
},
'B': {
'BAG', 'PURSE', 'CLUTCH',
'TOTE'
},
'A': {
'PINNI', 'BLOUSE', 'TOP',
'SKIRT', 'KNICKER', 'DRESS',
'DENIM', 'COAT', 'JACKET',
'SWEATER', 'JUMPER', 'SHIRT',
'SKINNY', 'SHORT', 'TEE',
'PANTS', 'JUMPSUIT', 'HIGH NECK',
'GOWN', 'TROUSER', 'ROBE',
'PLAYSUIT', 'CULOTTE', 'JODPHUR',
'PANTALON', 'FLARE', 'CARDIGAN',
'VEST', 'CAMI', 'BEDSHORT',
'PYJAMA', 'BRALET', 'TUNIC',
'HOODY', 'SATEEN', 'BIKER',
'JEAN', 'SWEAT', 'PULL',
'BIKINI', 'LE GRAND GARCON'
},
}

COLORS = {
'BLACK', 'WHITE', 'BLUE',
'YELLOW', 'ORANGE', 'GREY',
'PINK', 'FUSCIA', 'RED',
'GREEN', 'PURPLE', 'INDIGO',
'VIOLET'
}

class OxygendemoItem(scrapy.Item):
code = scrapy.Field()  # unique identifier (retailers perspective)
description = scrapy.Field()  # Detailed description
designer = scrapy.Field()  # manufacturer
eur_price = scrapy.Field()  # full (non_discounted) price
gender = scrapy.Field()  # F - Female, M - male
gbp_price = scrapy.Field()  # full (non_discounted) price
image_urls = scrapy.Field()  # list of urls representing the item
link = scrapy.Field()  # url of product page
name = scrapy.Field()  # short summary of the item
raw_color = scrapy.Field()  # best guess of color. Default = None
sale_discount = scrapy.Field()  # % discount for sale item where applicable
stock_status = scrapy.Field()  # list of sizes in stock
type = scrapy.Field()  # 'A' = apparel, 'B' = bags, 'S' = shoes, 'J' = jewelry, 'R' = accessories
usd_price = scrapy.Field()  # full (non_discounted) price

class OxygenSpider(scrapy.spiders.sitemap.SitemapSpider):
allowed_domains = ["oxygenboutique.com"]
sitemap_urls = ['http://www.oxygenboutique.com/sitemap.xml']
rules = (scrapy.spiders.Rule(
# Extract links in sitemap but not matching 'products.aspx', 'contactus.aspx'…
'Products\.aspx',
'ContactUs\.aspx',
'SearchResult\.aspx',
)),
callback='parse_product',
),)

def __init__(self, base_currency='GBP'):
self.ex_rates = get_exchange_rates(base_currency)
super(OxygenSpider, self).__init__()

def parse_product(self, response):
self.logger.info('Entered into parse_product method')
self.logger.debug('Respons status: {}'.format(response.status))

item = OxygendemoItem()
DOM = PyQuery(response.body)
# find that "add to shopping bag button" or we are not on a product page
return
base_url , item['code'] = parse_url(response.url)
description = DOM('.right div#accordion').children().eq(
1).text().encode('ascii', 'ignore')
summary_bag = set(description.upper().split())
item['description'] = description
item['type'] = self.type_of_product(summary_bag)
item['name'] = DOM('.right h2').text()

item['gbp_price'], item['sale_discount'] = price_and_discount(
DOM('.price').children())

if self.ex_rates is not None:
base_price = item['gbp_price']
for currency in ('USD','EUR'):
field = '{}_price'.format(currency.lower())
price = base_price * self.ex_rates[currency]
item[field] = '{0:.2f}'.format(price)
else:
item['usd_price'], item['eur_price'] = ['N/A'] * 2

item['designer'] = DOM('.right').find('.brand_name a').text()
item['stock_status'] = [
elem.text()
for elem in DOM('select').children()
if 'Sold Out' not in elem.text()]
item['gender'] = 'F'  # Oxygen boutique carries Womens's clothing
item['image_urls'] = [
urlparse.urljoin(base_url, image.attrib['src'])
for image in DOM('div#product-images tr td a img')]
item['raw_color'] = self.color_of_product(summary_bag)

return item

@staticmethod
def type_of_product(summary):
for category, items in PRODUCTS_TYPES.iteritems():
for item in items:
if item in summary:
return category
return 'R'  # Tag as accessory as failsafe

@staticmethod
def color_of_product(summary):
''' Will go this route to avoid external imports'''
try:
return (summary & COLORS).pop()
except KeyError:
return None

@staticmethod
def price_and_discount(gbp_price):
if not gbp_price('.mark').text():  # No discount
discount = '0%'
price = float(gbp_price.parent().text())
else:  # Calculate discount
price = float(gbp_price('.mark').text())
new_price = float(gbp_price.eq(1).text())
discount = '{0:.2f}%'.format(price / new_price * 100)
return price, discount

def parse_url(raw_url):
url = urlparse.urlparse(raw_url)
page_name = url.path.lstrip('/').rsplit('.',1)[0]
return '{}://{}'.format(url.scheme, url.netloc), page_name

def get_exchange_rates(base_currency):
'''return a dictionary of exchange rates compared to the base currency'''
url = 'http://api.fixer.io/latest?base={}'.format(base_currency)
try:
response = urllib2.urlopen(url)

• @trendsetter37 Also I have the feeling that the PRODUCTS_TYPES collection should be built out of a query to avoid the need of maintaining it when the site adds new categories for instance… – 301_Moved_Permanently Oct 24 '15 at 8:10