3
\$\begingroup\$

The following code takes either a URL or the title to an existing news article.

  1. Searches Google News using the title.

  2. Collects all links from search results.

import urllib2
from lxml import html
import requests

def get_page_tree(url=None):
    page = requests.get(url=url, verify=False)
    return html.fromstring(page.text)

def get_title(url=None):
    tree = get_page_tree(url=url)
    return tree.xpath('//title//text()')[0].strip().split(' -')[0]

def find_other_news_sources(url=None, title=None):
    # Google forwards the url using <google_domain>/url?q=    <actual_link>. This might change over time
    forwarding_identifier = '/url?q='
    if not title:
        title = get_title(url=url)
    google_news_search_url = 'http://www.google.com/search?q=' + urllib2.quote(title) + '&tbm=nws'
    google_news_search_tree = get_page_tree(url=google_news_search_url)
    other_news_sources_links = [a_link.replace(forwarding_identifier, '').split('&')[0] for a_link in
                            google_news_search_tree.xpath('//a//@href') if forwarding_identifier in a_link]
    return other_news_sources_links
\$\endgroup\$
-1
\$\begingroup\$

Why do all of your function arguments have default values of None? What purpose would these functions serve if they were run with all argument values as None?

Secondly, you should have two blank lines between your functions, as per PEP8.

Finally, I'd recommend adding docstrings to your functions, and your file as well. A typical function docstring looks something like this:

def my_func( ... ):
    """
    Put a description of your function
    and it's arguments here.
    """
    ...

Anyways, other than that, your code looks really nice! Keep up the good work!

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for your feedback. I agree with the None part. Unneccesary. Looking into PEP8 and docstring now. \$\endgroup\$ – user2573339 Jul 17 '15 at 18:15
3
\$\begingroup\$
  1. Instead of constructing the google_news_search_url with two string concatenations, use string formatting.
  2. The other_news_sources_links line is very dense. Please split it up.
  3. In addition to the defaults for all your args being None, you're calling all of your functions with keyword arguments, which seems unnecessary.
  4. But before you fix that, consider why you need these 2 line functions in the first place. They don't seem to do anything complicated enough to warrant having to jump around in the code.
\$\endgroup\$

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.