2
\$\begingroup\$

To be honest, I am pretty new to coding. I want to analyse the articles my local newspaper publishes on ther website. To do that, I have a list of URLs and from that, I want to download and save all the articles published. This is my code:

from bs4 import BeautifulSoup
import urllib.request
import pickle
import time

def savelist(allurls, dname):
    with open(dname, 'wb') as f:
        pickle.dump(allurls, f, pickle.HIGHEST_PROTOCOL)

def readlist(dname):
    with open(dname, 'rb') as f:
        return(pickle.load(f))

def turntosoup(liste):
    #turns a list of URLs to a list of soups
    soups = list()
    for url in liste:
        r = urllib.request.urlopen(url)
        soup = BeautifulSoup(r, 'html.parser')
        soups.append(soup)
    return(soups)

def einzelsuppe(url):
    r = urllib.request.urlopen(url)
    soup = BeautifulSoup(r, 'html.parser')
    return(soup)


def gettextdesk(soup):
    #extracts the Text from the Soup and returns a List
    article=(soup.find(class_='g_m_1of2'))
    try:
        hlwithhtml=article.find('h1')
        hl = hlwithhtml.string
    except:
        hl = ''

    try:
        pubdatewithhtml=article.find(class_='publication_date')
        pubdate = pubdatewithhtml.getText(' ', strip=True)
    except:
        pubdate = ''

    try:
        abstractwithhtml=article.find(class_='abstract ')
        abstract=abstractwithhtml.getText(' ', strip=True)
    except:
        abstract = ''

    try:
        contentwithhtml=article.find(class_='content')
        while contentwithhtml.script:
            contentwithhtml.script.decompose()
        content = contentwithhtml.getText(' ', strip=True)
    except:
        content = ''

    newspost = [str(hl), str(abstract), str(content), str(pubdate)]
    return(newspost)

def workonlist(urllist):
    souplist = turntosoup(urllist)
    soups = list()
    for soup in souplist:
        geparst = gettextdesk(soup)
        soups.append(geparst)
    return(soups)

start_time = time.time()

essenurls = readlist('Liste URLs Essen')

def chunker(seq, size):
    # found here: http://stackoverflow.com/questions/434287/what-is-the-most-pythonic-way-to-iterate-over-a-list-in-chunks
    return[seq[pos:pos + size] for pos in range(0, len(seq), size)]


def abarbeiten(essenurls):
    count = 50
    for teilurls in chunker(essenurls, 50):
        savelist(workonlist(teilurls), 'Essen %d' % count)
        count +=50

abarbeiten(essenurls)

print("--- %s seconds ---" % (time.time() - start_time))

Where should I change stuff to improve performance? It takes about 1.9 seconds per URL, and i wondered if I could make it faster.

This is the output of python3 -m cProfile -s cumtime Abrufen.py:

http://pastebin.com/2yLXGx3s (i couldn't post it here because of the character limit)

Here is a list of URLs as an example:

essenurls = ['http://www.derwesten.de/staedte/essen/schweinekopf-an-moschee-in-essen-altendorf-befestigt-id12200572.html','http://www.derwesten.de/staedte/essen/vor-drei-jahren-verschwand-pierre-pahlke-aus-essen-id12201302.html','http://www.derwesten.de/staedte/essen/essen-packt-an-geht-riesenbaerenklau-an-die-wurzel-id12201120.html']
\$\endgroup\$
  • \$\begingroup\$ Please note that German language variable/function names makes it way harder for us to review your code. Also including the output of python3 -m cProfile -s cumtime read_articles.py will help potential reviewers to see which functions take the most time and should therefore be more optimized \$\endgroup\$ – Caridorc Sep 17 '16 at 19:59
1
\$\begingroup\$

Repetition and potential for bug hiding

try:
    hlwithhtml=article.find('h1')
    hl = hlwithhtml.string
except:
    hl = ''

try:
    pubdatewithhtml=article.find(class_='publication_date')
    pubdate = pubdatewithhtml.getText(' ', strip=True)
except:
    pubdate = ''

try:
    abstractwithhtml=article.find(class_='abstract ')
    abstract=abstractwithhtml.getText(' ', strip=True)
except:
    abstract = ''

try:
    contentwithhtml=article.find(class_='content')
    while contentwithhtml.script:
        contentwithhtml.script.decompose()
    content = contentwithhtml.getText(' ', strip=True)
except:
    content = ''

Potential for bug hiding

except without argument swallows any exception, that is if you mistype for example hlithhtml, h1 will always be empty and you will not be notified of any error.

Repetition

You try an action and if it fails resort to a default value. Also these blocks of code

    pubdatewithhtml=article.find(class_='publication_date')
    pubdate = pubdatewithhtml.getText(' ', strip=True)

    abstractwithhtml=article.find(class_='abstract ')
    abstract=abstractwithhtml.getText(' ', strip=True)

Are very similar only class_ changes.

I suggest you re-organize this code using:

def action_with_default(action, exception, default):
    try:
        return action()
    except exception:
        return default


def find_class(article, class_):
    x = article.find(class_=class_)
    return x.getText(' ', strip=True)

This should save you a lot of duplication.

\$\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.