4
\$\begingroup\$

This accepts text and returns a list of all words which occur with a required frequency (default frequency is 2). It works well, but gets too slow with large texts, more than 1M words. Where can this function be improved to speed searches in larger text?

from collections import Counter
import re

def words_in_text_counter(in_text,frequency=2):
    no_noise_text = re.sub(r'[0-9.,:;<>"#!%&/()=?*-+]','',in_text.lower()).strip().split()
    frequency_dict = {}
    for key, value in Counter(no_noise_text).items():
        frequency_dict.setdefault(value, []).append(key)
    try:
        print(str(frequency)+' times words in text.')
        return frequency_dict[frequency]
    except KeyError:
        return str(frequency)+' times never happens in text.'
\$\endgroup\$
  • \$\begingroup\$ Do you want the full frequency_dict returned, or only those word with a given frequency, or above a given frequency? Kind of doesn't make sense to just return the words at a given frequency, and then having to rerun a second time if you want to check for another frequency... \$\endgroup\$ – holroy Sep 14 '15 at 17:14
3
\$\begingroup\$

Your problem is ultimately:

no_noise_text = re.sub(r'[0-9.,:;<>"#!%&/()=?*-+]','',in_text.lower()).strip().split()

There's a lot to dislike about this line. It horribly violates the Law of Demeter. It takes a while to figure out what it is supposed to do. It uses the wrong tool for the job. And it's inefficient.

First, to start with, if the goal is to split a strict by non-alpha characters, then we should just use the correct function: re.split:

words = re.split("[^a-z']+", in_text.lower())
counts = Counter(words)

Next, this is going to be inefficient. We have to walk the entire string once (to make it lower-case), then again to split it. And then we have to keep all of the words in memory. We don't have to do any of that if we really start with the correct function: re.finditer:

re.finditer("([a-zA-Z']+)", in_text)

That will give you an iterator over the words. Which you can stick into a generator expression that you can pass to Counter:

words = re.finditer("([a-zA-Z']+)", in_text)
counts = Counter(m.group(0).lower() for m in words)

And if you're already using Counter, you may as well use defaultdict:

freq_dict = defaultdict(list)
for key, val in counts.iteritems():
    freq_dict[key].append(val)

Note that I'm using iteritems() (which just gives you a generator) rather than items() (which must give you the full list).

Furthermore, why do we need a frequency_dict? We only care about those words which match the given frequency. Why not just keep a list?

frequency_list = []
for key, val in counts.iteritems():
    if val == frequency:
        frequency_list.append(key)

Putting it all together

from collections import Counter
import re

def words_in_text_counter(in_text, frequency=2):
    words = re.finditer("([a-zA-Z']+)", in_text)
    counts = Counter(m.group(0).lower() for m in words)

    frequency_list = []
    for key, val in counts.iteritems():
        if val == frequency:
            frequency_list.append(key)

    if frequency_list:
        print(str(frequency)+' times words in text.')
        return frequency_list
    else:
        return str(frequency)+' times never happens in text.'
\$\endgroup\$
  • 1
    \$\begingroup\$ I'm kind of unsure what the OP really wants, but wouldn't it make more sense to actually return the frequency_dict, and then provide some simpler map function to get either the word list for a given frequency, or frequencies above a given level? \$\endgroup\$ – holroy Sep 14 '15 at 17:16
  • \$\begingroup\$ Thanks Barry it is much faster now. I was unable activate counts.iteritems() because iteritems() method not available in Counter and use items() instead \$\endgroup\$ – mmachine Sep 14 '15 at 17:54
  • \$\begingroup\$ .iteritems() is only available in Python 2.x; the OP has tagged the question with python3 which means that .items() is fine (and the only approach, afaik) \$\endgroup\$ – alexwlchan Sep 14 '15 at 21:54

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.