from urllib import urlopen
from gzip import GzipFile
from cStringIO import StringIO
import re
import urllib
import urllib2
import webbrowser
import mechanize
import time
from difflib import SequenceMatcher
import os
def download(url):
s = urlopen(url).read()
if s[:2] == '\x1f\x8b': # assume it's gzipped data
with GzipFile(mode='rb', fileobj=StringIO(s)) as ifh:
s = ifh.read()
return s
I'd probably pick a more descriptive name then s
s = download('http://www.locationary.com/place/en/US/Utah/Provo-page5/?ACTION_TOKEN=NumericAction')
findLoc = re.compile('http://www\.locationary\.com/place/en/US/.{1,50}/.{1,50}/.{1,100}\.jsp')
For constants, its usually best practice to name them in ALL_CAPS and put them outside of the actual logic. The logic in your program should really be in a main function not at the module level.
findLocL = re.findall(findLoc,s)
Not a very descriptive name, perhaps you can come up with a better one.
W = []
X = []
XA = []
Y = []
YA = []
Z = []
ZA = []
Don't double-space your code. Also, these variables are either really badly name or shouldn't be seperate variables. Possibly both.
for i in range(0,25):
Why 25? Do you want everything in findLocL? Then you should use for loc in findLocL
.
b = download(findLocL[i])
findYP = re.compile('http://www\.yellowpages\.com/')
findYPL = re.findall(findYP,b)
You don't have to call compile, you can just pass the regular expression as the first parameter. Compiling is only helpful if you use the compiled expression multiple times. Also, aren't you just searching for a simple string here? Why use regular expressions?
findTitle = re.compile('<title>(.*) \(\d{1,10}.{1,100}\)</title>')
getTitle = re.findall(findTitle,b)
findAddress = re.compile('<title>.{1,100}\((.*), .{4,14}, United States\)</title>')
getAddress = re.findall(findAddress,b)
if not findYPL:
if not getTitle:
Why did you findall if you just endup checking for a single match?
print ""
else:
W.append(findLocL[i])
What the fried monkey is W? What you are searching for? You code is difficult to follow.
b = download(findLocL[i])
if not getTitle:
print ""
else:
X.append(getAddress)
b = download(findLocL[i])
So you just downloaded this file a few lines before. Why are you downloading it again? And why don't you ever do anything with the b you get?
if not getTitle:
print ""
else:
Y.append(getTitle)
sizeWXY = len(W)
Was that really helpful?
def XReplace(text, dic):
for i, j in dic.iteritems():
text = text.replace(i, j)
XA.append(text)
def YReplace(text2, dic2):
for k, l in dic2.iteritems():
text2 = text2.replace(k, l)
YA.append(text2)
These functions do pretty much the same thing. The only difference is that they both modify global variables. You shouldn't really be modifying global variables anyways.
for d in range(0,sizeWXY):
old = str(X[d])
So... sizeWXY is len(W) but here you are fetching from X, what the fried turkey?
reps = {' ':'-', ',':'', '\'':'', '[':'', ']':''}
XReplace(old, reps)
old2 = str(Y[d])
YReplace(old2, reps)
count = 0
for e in range(0,sizeWXY):
newYPL = "http://www.yellowpages.com/" + XA[e] + "/" + YA[e] + "?order=distance"
v = download(newYPL)
abc = str('<h3 class="business-name fn org">\n<a href="')
Why are constructing a string from a string?
dfe = str('" class="no-tracks url "')
findFinal = re.compile(abc + '(.*)' + dfe)
getFinal = re.findall(findFinal, v)
if not getFinal:
W.remove(W[(e-count)])
Use del W[e-count]
X.remove(X[(e-count)])
count = (count+1)
No need for those parens
else:
for f in range(0,1):
Z.append(getFinal[f])
So... that loop only executes once. Why is it a loop?
XA = []
Rather then reusing XA, I'd suggest working with a new list here.
for c in range(0,(len(X))):
aGd = re.compile('(.*), .{1,50}')
bGd = re.findall(aGd, str(X[c]))
bGd? seriously? What the fried turkey marshmallow duck is that supposed to be?
XA.append(bGd)
LenZ = len(Z)
Don't do this. You are probably doing this because if you don't in C, it'll recalculate the length constantly as you execute a loop. Python doesn't do that.
V = []
for i in range(0,(len(W))):
Here you do pass the length to range. But you don't need those parens around it.
if i == 0:
countTwo = 0
Why don't you do this before the loop starts? That should be the same
gda = download(Z[i-(countTwo)])
No need for the parens around countTwo.
ab = str('"street-address">\n')
cd = str('\n</span>')
ZAddress = re.compile(ab + '(.*)' + cd)
ZAddress2 = re.findall(ZAddress, gda)
for b in range(0,(len(ZAddress2))):
In python you should almost never use a range loop unless you are actually counting. If you are looping with a length of something, you should almost always be looping over the list not range.
if not ZAddress2[b]:
print ""
else:
V.append(str(ZAddress2[b]))
a = str(W[i-(countTwo)])
n = str(Z[i-(countTwo)])
c = str(XA[i])
d = str(V[i])
So many hopelessly badly named variables. It hurts!
#webbrowser.open(a)
#webbrowser.open(n)
m = SequenceMatcher(None, c, d)
Ok, I'm impressed that you know about and use this.
if m.ratio() < 0.50:
Z.remove(Z[i-(countTwo)])
W.remove(W[i-(countTwo)])
countTwo = (countTwo+1)
#LenZ2 = LenZ - (len(Z))
#if len(Z) == LenZ:
#print "All of the Yellowpages were correct!"
#else:
#if LenZ2 > 1:
#print str(LenZ2) + " of the Yellowpages were incorrect!"
#else:
#print str(LenZ2) + " of the Yellowpages was incorrect!"
Don't keep dead code in comments
def ZReplace(text3, dic3):
for p, q in dic3.iteritems():
text3 = text3.replace(p, q)
ZA.append(text3)
Deja vu!
for y in range(0,len(Z)):
old3 = str(Z[y])
reps2 = {':':'%3A', '/':'%2F', '?':'%3F', '=':'%3D'}
ZReplace(old3, reps2)
When you find yourself repeating the same code several times, its a sign you need to refactor
#br = mechanize.Browser()
for z in range(0,len(ZA)):
findPID = re.compile('\d{5,20}')
getPID = re.findall(findPID,str(W[z]))
newPID = re.sub("\D", "", str(getPID))
finalURL = "http://www.locationary.com/access/proxy.jsp?ACTION_TOKEN=proxy_jsp$JspView$SaveAction&inPlaceID=" + str(newPID) + "&xxx_c_1_f_987=" + str(ZA[z])
webbrowser.open(finalURL)
time.sleep(5)
os.system("taskkill /F /IM firefox.exe")
Stylistically, your code has a few issues which I've mentioned above. But in terms of performance, you are downloading web pages. The downloads will take way longer then the code so any speed issues in your code itself will not be noticeable.
I did point a couple of cases where you are downloading files you don't actually do anything with. Getting rid of those will help your code be faster.
You are trying to pull things out of HTML using regular expressions. This is a bad idea. It'll be brittle, hard to get right, and hard to read. Instead you should use something like BeautifulSoup: http://www.crummy.com/software/BeautifulSoup/. It'll parse HTML, (even broken HTML) and make it easy to pull out the pieces you are interested in.
The way to speed up the code is to fetch multiple web pages at the same time, and to process once web page while waiting for the others to download. The easiest way to do this is with the eventlet library.
Here is my rewrite of your code. I make no guarantees of correctness and I know it doesn't do the error handling it should. It also has too much stuff going on in the main function. But it doesn't suffer from the same speed problems. I'm just trying to give a hint at how your code could be easier to follow and not have the same performance problems.
from gzip import GzipFile
from cStringIO import StringIO
import re
import webbrowser
import time
from difflib import SequenceMatcher
import os
import sys
from BeautifulSoup import BeautifulSoup
import eventlet
from eventlet.green import urllib2
START_URL = 'http://www.locationary.com/place/en/US/Utah/Provo-page5/?ACTION_TOKEN=NumericAction'
TITLE_MATCH = re.compile(r'(.*) \(\d{1,10}.{1,100}\)$')
ADDRESS_MATCH = re.compile(r'.{1,100}\((.*), .{4,14}, United States\)$')
LOCATION_LISTING = re.compile(r'http://www\.locationary\.com/place/en/US/.{1,50}/.{1,50}/.{1,100}\.jsp')
def download(url):
print "Downloading:", url
s = urllib2.urlopen(url).read()
if s[:2] == '\x1f\x8b': # assume it's gzipped data
ifh = GzipFile(mode='rb', fileobj=StringIO(s))
s = ifh.read()
print "Downloaded: ", url
return s
def replace_chars(text, replacements):
return ''.join(replacements.get(x,x) for x in text)
def handle_listing(listing_url):
listing_document = BeautifulSoup(download(listing_url))
# ignore pages that link to yellowpages
if not listing_document.find("a", href=re.compile(re.escape("http://www.yellowpages.com/") + ".*")):
listing_title = listing_document.title.text
reps = {' ':'-', ',':'', '\'':'', '[':'', ']':''}
title, = TITLE_MATCH.match(listing_title).groups()
address, = ADDRESS_MATCH.match(listing_title).groups()
yellow_page_url = "http://www.yellowpages.com/%s/%s?order=distance" % (
replace_chars(address, reps),
replace_chars(title, reps),
)
yellow_page = BeautifulSoup(download(yellow_page_url))
page_url = yellow_page.find("h3", {"class" : "business-name fn org"})
if page_url:
page_url = page_url.a["href"]
business_name = title[:title.index(",")]
page = BeautifulSoup(download(page_url))
yellow_page_address = page.find("span", {"class" : "street-address"})
if yellow_page_address:
if SequenceMatcher(None, address, yellow_page_address.text).ratio() >= 0.5:
pid, = re.search(r'p(\d{5,20})\.jsp', listing_url).groups(0)
page_escaped = replace_chars(page_url, {':':'%3A', '/':'%2F', '?':'%3F', '=':'%3D'})
final_url = "http://www.locationary.com/access/proxy.jsp?ACTION_TOKEN=proxy_jsp$JspView$SaveAction&inPlaceID=%s&xxx_c_1_f_987=%s" % (
pid, page_escaped)
return final_url
def main():
pool = eventlet.GreenPool()
listings_document = BeautifulSoup(download(START_URL))
listings = listings_document.findAll("a", href = LOCATION_LISTING)
listings = [listing['href'] for listing in listings]
for final_url in pool.imap(handle_listing, listings):
print final_url
if __name__ == '__main__':
main()