I get a lot done with Python scripts but always feel like I'm working with a messy desk when I'm using it.  I assume this is because I am not coding in Python correctly.  Coming from a C# and Ruby background, I've used Python mainly as a utility language writing together things that I need quickly - never really trying to refine my work - until now.I want criticism as to what I'm doing that's making my Python code feel sloppy - I feel I will advance more quickly if I have such feedback, and, since I don't have a Python mentor, here I am.The below script scrapes new product data from an online store.  I typically run this script from my console simply by typing python my_scraper_script.py and it will scrape the data and upload it to my rails site that I have running locally (this is a hobby project of mine).Some initial concerns:Does it make sense to declare some of the methods here in this script and some of the methods in another script (scraper_tools) when some of those methods are shared by similar scripts?My websiteProduct class does all of the work in the initializer part of the class, this feels dirty to me.I have a bunch of variables declared in what feels like an arbitrary position within the file.  Is this acceptable?The part where this script actually gets started doing things is just hanging out in the open somewhere arbitrarily toward the end of the actual file, is this normal?I'm sure there's much more awry - please fix me.import jsonimport reimport urllib2from scraper_tools import soupify, getHtml, buildJsonPostRequest__author__ = 'ecnalyr'def getImageLinkFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        fullSrc = div.find('img')['src']        return fullSrc.replace(//, http://) # removes the unnecessary // from the beginning of the string    except AttributeError:        return There is no Image Linkdef getSkuFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        baseUrl = div.find('a')['href']        baseSku = re.search('\productId=(\d+_\d+)', baseUrl).group(0)        return baseSku.replace(productId=, )    except AttributeError:        return There is no Skudef encodeBrandNameToUTF8(brandAndName):    return str(brandAndName.encode(utf-8))def getBrandAndNameFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        brandAndName = div.find('span', {'class': 'name'}).string        return str(brandAndName)    except UnicodeEncodeError:        return encodeBrandNameToUTF8(brandAndName)    #        return unicode encode error    except AttributeError:        return There is no brand and / or namedef getPriceFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        price = div.find('span', {'class': 'price'}).string        return price.replace($, ) # have to remove the dollar sign from the price    except AttributeError:        return There is no pricedef buildLinkFromSKU(sku):    Expects a SKU representing a product at {website}    return productBaseUrl + skuclass websiteProduct:    A product from {website's} created using a newItemDiv in a BeautifulSoup format    def __init__(self, productDiv):        self.price = str(getPriceFromDiv(productDiv))        self.brandAndName = str(getBrandAndNameFromDiv(productDiv))        self.sku = str(getSkuFromDiv(productDiv))        self.imageLink = str(getImageLinkFromDiv(productDiv))        self.link = str(buildLinkFromSKU(self.sku))        self.json = json.dumps({'store':Website Name, 'name':self.brandAndName, 'price':self.price, 'sku':self.sku,                                'link':self.link, 'imageLink':self.imageLink})    def getPrice(self):        return self.price    def getBrandAndName(self):        return self.brandAndName    def getLink(self):        return self.link    def getImageLink(self):        return self.imageLink    def getSku(self):        return self.sku    def getJson(self):        return self.jsonuploadProductsUrl = http://MySite.com/products.jsonwebsiteNewItemLink = http://www.website.com/newItemsnewItemSoup = soupify(getHtml(websiteNewItemLink))newItems = newItemSoup.find_all('div', {'class': 'productNew'})productBaseUrl = http://www.website.com/product?productId=productList = []for item in newItems:    product = websiteProduct(item)    productList.append(product)for item in productList[0:]:    jsonData = item.getJson()    request = buildJsonPostRequest(uploadProductsUrl)    urllib2.urlopen(request, jsonData)
Scraping new product data from an online store
python;web scraping
import jsonimport reimport urllib2from scraper_tools import soupify, getHtml, buildJsonPostRequest__author__ = 'ecnalyr'def getImageLinkFromDiv(div):The python style guide recommends lowercase_with_underscores for function names    Expects a BeautifulSoup div from {website's} new product page    try:        fullSrc = div.find('img')['src']Python convention is for local variables to be lowercase_with_underscores        return fullSrc.replace(//, http://) # removes the unnecessary // from the beginning of the stringThis will also make replacements throughout the entire string, not just the beginning. It's also not really removing it from the beginning. It also seems like a more generic thing and perhaps should be in a seperate normalize_url function.    except AttributeError:It's rarely a good idea to catch an AttributeError, python will produce that errors for simple typos in your code so catching it here is likely to mask real bugs. I'm not sure which attribute access you are catching, but I'm guess its that fullSrc can be None? In that case you should really check if fullSrc is None:        return There is no Image LinkThat's a terrible way to report an error. Either you should return None or an empty string or raise an exception.  But returning a string with an error message is bad because the rest of the program won't treat it as special.def getSkuFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        baseUrl = div.find('a')['href']        baseSku = re.search('\productId=(\d+_\d+)', baseUrl).group(0)You should use .group(1) as that will return the part in the parens. Then you don't need to replace trick.        return baseSku.replace(productId=, )    except AttributeError:        return There is no Skudef encodeBrandNameToUTF8(brandAndName):    return str(brandAndName.encode(utf-8)).encode() already returns a string, you don't need to stringify it again. The function also implies its specific to brand names, but its not.def getBrandAndNameFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        brandAndName = div.find('span', {'class': 'name'}).string        return str(brandAndName)    except UnicodeEncodeError:        return encodeBrandNameToUTF8(brandAndName)Just return encodeBrandNameToUTF8(brandAndName) the first time. You don't gain anything by trying and failing and trying again.     #        return unicode encode error    except AttributeError:        return There is no brand and / or namedef getPriceFromDiv(div):    Expects a BeautifulSoup div from {website's} new product page    try:        price = div.find('span', {'class': 'price'}).stringThis line is very similiar to the line in the previous function. I'd write a function to share the similiarties.        return price.replace($, ) # have to remove the dollar sign from the priceI don't like this because it implies that you are removing it from across the whole string rather then just the beginning. Instead I'd use return price.lstrip('$')    except AttributeError:        return There is no pricedef buildLinkFromSKU(sku):    Expects a SKU representing a product at {website}    return productBaseUrl + skuclass websiteProduct:Python style guide recomend CamelCase for class names    A product from {website's} created using a newItemDiv in a BeautifulSoup format    def __init__(self, productDiv):        self.price = str(getPriceFromDiv(productDiv))        self.brandAndName = str(getBrandAndNameFromDiv(productDiv))Python style guide recommends lowercase_with_underscores for object attributes.        self.sku = str(getSkuFromDiv(productDiv))        self.imageLink = str(getImageLinkFromDiv(productDiv))        self.link = str(buildLinkFromSKU(self.sku))Stylistically, it'd make more sense for your get function to convert to strings. Especially since many of them already do.        self.json = json.dumps({'store':Website Name, 'name':self.brandAndName, 'price':self.price, 'sku':self.sku,                                'link':self.link, 'imageLink':self.imageLink})    def getPrice(self):        return self.price    def getBrandAndName(self):        return self.brandAndName    def getLink(self):        return self.link    def getImageLink(self):        return self.imageLink    def getSku(self):        return self.sku    def getJson(self):        return self.jsonIn python, we do not have get functions. Just access the attributes directly. If you need more complicated getter logic, you can use properties.uploadProductsUrl = http://MySite.com/products.jsonwebsiteNewItemLink = http://www.website.com/newItemsnewItemSoup = soupify(getHtml(websiteNewItemLink))newItems = newItemSoup.find_all('div', {'class': 'productNew'})productBaseUrl = http://www.website.com/product?productId=Some of these are global constants, they should be named in ALL_CAPS, and moved to the top of the file. Other ones shouldn't really be global, and those should  become local variables in a main() function.productList = []for item in newItems:    product = websiteProduct(item)    productList.append(product)You can use productList = map(websiteProduct, newItems), or productList = [websiteProduct(item) for item in newItems] or at least combine the last two lines.for item in productList[0:]:Why the [0:]? it does nothing    jsonData = item.getJson()    request = buildJsonPostRequest(uploadProductsUrl)    urllib2.urlopen(request, jsonData)I recommend actually looking at the response to make sure it at least gave a sucesfull status.Does it make sense to declare some of the methods here in this script  and some of the methods in another script (scraper_tools) when some of  those methods are shared by similar scripts?Absolutely. In fact I'd move anything which looks like it could be generic into there.My websiteProduct class does all of the work in the initializer part  of the class, this feels dirty to me.For the purpose of this script, the whole class is pointless, because all you really want is to make the json. So I'd just have a function that takes the div and returns the json. In general, having the constructor fetching attributes like that will loose flexibility.I have a bunch of variables declared in what feels like an arbitrary  position within the file. Is this acceptable?Not really. Global constants should be at the top. Non-globals should be in functions.The part where this script actually gets started doing things is just  hanging out in the open somewhere arbitrarily toward the end of the  actual file, is this normal?Normal yes, correct no. You should put all that stuff in a main() function and add this:if __name__ == '__main__':     main()That will only execute the main function when the script is executed directly. You can import it without executing it which may be useful. Now, thats a bit of a purist answer. For trivial scripts (this one is skirting on the boundary) I'll sometimes ignore that and just write it like you've done. 