9
\$\begingroup\$

I'm still fairly new to Python and web-scraping, but a colleague asked asked me if I could build a web-scraper that could be used by a think tank he's a member of to find news articles matching a pre-defined list of trigger words of interest to the organisation.

The purpose of the following script is as follows:

  1. Parse an RSS feed of news articles published by a British news outlet
  2. Read each news article listed in the feed by accessing the link for each item in the RSS feed.
  3. Search each article for pre-defined words and phrases
  4. Write the results (matching trigger word/phrase and link to article) to a CSV.

Here's the code:

import feedparser
from bs4 import BeautifulSoup
from lxml import html
import requests
import re

import csv

words = ['divorce', 'custody battle', 'meal ticket', 'behind closed doors', 'detail of the case emerged on a legal database', ]

hit_article = []
links_list = []
hits = []
hit_link = []

d = feedparser.parse('http://www.dailymail.co.uk/articles.rss')

for item in d.entries:
    link = ( item[ "link" ] )
    links_list.append(link)
    title = ( item ["title"])


for link in links_list[:100]:
    page = requests.get(link)
    tree = html.fromstring(page.content)

    soup = BeautifulSoup(page.text, 'html.parser')

    text = soup.find('body')
    text = text.text

    for word in words:
        regex = r"\b"+ re.escape(word) + r"\b"
        match = re.search(regex, text)
        if match:
            print (word, ' ', title, "________found")
            hits.append(word)
            hit_link.append(str(link))

match_dictionary = dict(zip(hit_link, hits))

print (match_dictionary)

w = csv.writer(open("output.csv", "w"))
for key, val in match_dictionary.items():
    w.writerow([key, val])

The code does what I want it to do, but I know it stinks (particularly where I fall back into two for loops.

How can I achieve the same result more cleanly and Pythonically?

\$\endgroup\$
9
\$\begingroup\$

At first glance, I'd say that this code isn't bad. I am particularly impressed by this line:

regex = r"\b"+ re.escape(word) + r"\b"

because you took the care to escape the word, and to ensure that it is starts and ends at word boundaries. (You might want to do case-insensitive searches, though? Also, you might want to treat all whitespace as equivalent, in case a newline occurs in the middle of a phrase.)

You do have a bug, though, in that the on-screen output reports all hits as coming from the last title in the RSS feed.

Code organization

This program is starting to be long enough that you should break it up into functions. In particular, when the code gets long, all of the variables (hit_article, links_list, hits, hit_link, d, etc.) act as global variables, making it hard to keep track of how they are used. That is the root cause of your title bug.

Wasted work

The silliest mistake is that tree = html.fromstring(page.content) is never used, so you used the lxml library to parse the HTML a second time for no reason.

Less obviously, you have a problem if one article contains multiple search terms. The on-screen printout will report all terms that were found. However, when you do dict(zip(hit_link, hits)), you will only store the last hit per link. You'll need to decide whether you want to:

  • report all search terms that are found in each article (in which case you need to modify the data structure to store more results, or eliminate the dictionary entirely)
  • report only the first search term in your list that appears in each article (in which case you can break from the if match: block)
  • report only the first occurrence in the article of any of the search terms (in which case you should construct the regex to look for any of the search terms: regex = r'\b(?:' + '|'.join(re.escape(word) for word in words) + ')\b', instead of looping)

Comprehensions

In general, whenever you see the pattern:

output_list = []
for item in input_list:
     output_list.append(transform(item))

… you can write it more elegantly using a list comprehension.

For example, instead of:

list_list = []
d = feedparser.parse('http://www.dailymail.co.uk/articles.rss')
for item in d.entries:
    link = ( item[ "link" ] )
    links_list.append(link)

You should write (with a bit of renaming for clarity):

 rss_url = …
 links = [entry['link'] for entry in feedparser.parse(rss_url).entries]

Suggested solution

Here, I've elected to report all terms found in each article, by eliminating the match_dictionary altogether. I've used two generator functions.

from bs4 import BeautifulSoup
import csv
import feedparser
import re
import requests

def search_article(url, phrases):
    """
    Yield all of the specified phrases that occur in the HTML body of the URL.
    """
    response = requests.get(url)
    text = BeautifulSoup(response.text, 'html.parser').find('body').text
    for phrase in phrases:
        if re.search(r'\b' + re.escape(phrase) + r'\b', text):
            yield phrase

def search_rss(rss_entries, phrases):
    """
    Search articles listed in the RSS entries for phases, yielding
    (url, article_title, phrase) tuples.
    """
    for entry in rss_entries:
        for hit_phrase in search_article(entry['link'], phrases):
            yield entry['link'], entry['title'], hit_phrase

def main(rss_url, phrases, output_csv_path, rss_limit=None):
    rss_entries = feedparser.parse(rss_url).entries[:rss_limit]
    with open(output_csv_path, 'w') as f:
        w = csv.writer(f)
        for url, title, phrase in search_rss(rss_entries, phrases):
            print('"{0}" found in "{1}"'.format(phrase, title))
            w.writerow([url, phrase])

if __name__ == '__main__':
    rss_url = 'http://www.dailymail.co.uk/articles.rss'
    phrases = ['divorce', 'custody battle', …]
    main(rss_url, phrases, 'output.csv', 100)
\$\endgroup\$
  • \$\begingroup\$ Wow! Thank you for such a comprehensive and clearly explained review of my code. There are a few concepts in your revised version that are new to me, which is all the better! Thank you for taking the time to do this. \$\endgroup\$ – DanielH Feb 17 '18 at 18:10
  • \$\begingroup\$ I know this post is old. I tried the code results manually and noticed that the words are not only found in the body text. If you check where 'divorce' appears, you'll notice that they appear on the right hand column of each news, in 'related articles'. Is it a way to ommit these findings? \$\endgroup\$ – galtor Oct 13 at 8:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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