Style
Please read PEP8 and use a consistent style throughout your code:
- put a space after comas;
- use a linebreak after colons;
- use UPPERCASE names for constants…
Using locks
First off, locks support the context manager protocol and you can thus simplify your printer
to:
def printer(lock, data):
with lock:
print(data)
which may not warant a method on its own.
But most importantly, you say that
I've used locking within it to prevent two processes from changing its internal state.
but you're not changing any shared state at all. All you are doing with this lock is preventing outputs to be mismatched on the screen. Let's take a look at a modified version of your script: I’ve stored the process started so I can join
them and I print itemstorage
after all computation is done.
if __name__ == '__main__':
lock = Lock()
processes = [
Process(target=get_info, args=(link.format(page), lock, itemstorage))
for page in range(1, 15)
]
for p in processes:
p.start()
for p in processes:
p.join()
print('itemstorage is', itemstorage)
This prints
[…actual results snipped…]
itemstorage is []
This is because each process is operating on its own copy of itemstorage
and nothing is done to retrieve data afterward. Instead, you should have your processes return
the result and store them in itemstorage
yourself. In fact, this very process is already implemented using multiprocessing.Pool.map
.
Simplifying element retrieval
Since you extract text from the dom 3 times per title
, you can extract an helper function to simplify that task. Doing so, it will be even easier to build the return list using a list-comprehension:
def extract(element, descriptor, default=None):
try:
return element.cssselect(descriptor)[0].text
except IndexError:
if default is None:
raise
return default
def get_info(url):
response = requests.get(url).text
tree = fromstring(response)
return [(
extract(title, "a.business-name span"),
extract(title, "span.street-address", ""),
extract(title, "div[class^=phones]", ""),
) for title in tree.cssselect("div.info")]
This changes a bit the structure but I believe it is an improvement to better access the information. You can still use itertools.chain.from_iterable
if need be to flatten the returned list.
Proposed improvements
import itertools
from multiprocessing import Pool
import requests
from lxml.html import fromstring
LINK = "https://www.yellowpages.com/search?search_terms=coffee&geo_location_terms=Los%20Angeles%2C%20CA&page={}"
def extract(element, descriptor, default=None):
try:
return element.cssselect(descriptor)[0].text
except IndexError:
if default is None:
raise
return default
def get_info(url):
response = requests.get(url)
tree = fromstring(response.content)
return [(
extract(title, "a.business-name span"),
extract(title, "span.street-address", ""),
extract(title, "div[class^=phones]", ""),
) for title in tree.cssselect("div.info")]
if __name__ == '__main__':
pages_count = 14
with Pool(processes=pages_count) as pool:
urls = [LINK.format(page) for page in range(1, pages_count + 1)]
itemstorage = pool.map(get_info, urls)
for result in itertools.chain.from_iterable(itemstorage):
print(result)
Note that I also changed the document parsing part. For one lxml
is perfectly capable of handling bytes
so you don't have to perform decoding yourself; for two decoding into a string blindly can lead to using an improper charset, which lxml
can handle by looking into the appropriate meta
tag.