# Python function to find and tokenize string data

#!/usr/bin/env python

import re
import nltk.corpus

def tokenize(text, **kw):
"""
Break down text into a set of unique lower case words.
"""
plain_words = nltk.corpus.abc.words()
stop_words = nltk.corpus.stopwords.words()
# Defaults
kw.setdefault('min_len', 4)
kw.setdefault('max_len', 0)
kw.setdefault('ignore_list', [])
kw['ignore_list'] += stop_words

ok_length = lambda token: [len(token) >= kw['min_len']
and kw['max_len'] <= kw['max_len']][0]

tokens = set([token.lower()
for token in re.findall('([A-z]+)+', text)
if ok_length(token) and
token not in kw['ignore_list']]).intersection(plain_words)

## TESTING tokenize

print tokenize(TEXT,
min_len=4,
max_len=0,
ignore_list=['code'])


My concerns:

1. Is tokenize() too complex as one function?
2. Should I replace the list comprehensions?
3. Does this regex '([A-z]+)+' need improving?
4. How could I make this code more idiomatic?
5. Have I overlooked any faults in any of the logic?

Example output:

11:26 PM\$ python Tokenize.py


1. No, tokenize is not too complex for one function. It does a single operation: it tokenizes a string.
2. Your list comprehensions are fine. They are somewhat complex but are still readable and easily understandable.
3. Instead of using the regex ([A-z]+)+ you could simply use the 'shortcut' \w+'. This regex will match one-to-many words.

If you wanted your regex to recognize hyphenated words, a simple change will suffice:

\w(-?\w)*

4. Your code is quite Pythonic. Your spacing and indentation is fine. The same goes for your variable names. If you have more concerns about style, consult PEP8, the official Python style guide.
5. There is a flaw in your logic though. Take your lambda expression:

ok_length = lambda token: [len(token) >= kw['min_len']
and kw['max_len'] <= kw['max_len']][0]


Currently, this will except any string whose length is greater than the min_length this is because you have kw['max_len'] == kw['max_len'] which will always evaluate True. You also have the conditionals switched around. This is how you should have the check:

ok_length = lambda token: [kw['min_length'] <= len(token) <= kw['max_length']][0]


Now, the question you need to ask is, whether or not it pays off to use a lambda expression here. Is the functionality 'deserving' of a lambda expression? Or can the check simply be placed into the list comprehension a little later on? Here's what it would look like, I'll let you decide (I like this way):

tokens = set([token.lower()
for token in re.findall('([A-z]+)+', text)
if kw['min_len'] <= len(token) <= kw['max_len'] and
token not in kw['ignore_list']]).intersection(plain_words)

6. The next thing I would consider is your used of the kw dict. How needed is it? Could you just implement optional parameters in the function declaration? I like that implementation because this function does not (and probably won't) take an arbitrary number of keyword arguments. It will only take the 3. As a related side note, you have swapped your max_len and min_len values. Currently, max_len < min_len.

7. Finally, I would remove the intersection function call when you assign the tokens variable. The name tokens and the context with which its generated implies that tokens will hold all tokens that match the given regex. However, in your case, it only contains that are also in plain_words. I would wait to do the intersection until the return statement:

return tokens.intersection(plain_words)


Here is 'my' version of the tokenize function:

def tokenize(text, min_len=0, max_len=4, ignore_list=[]):
"""
Break down text into a set of unique lower case words.
"""
plain_words = nltk.corpus.abc.words()
stop_words = nltk.corpus.stopwords.words()
ignore_list += stop_words

tokens = set([token.lower()
for token in re.findall('\w+', text)
if min_len <= len(token) <= max_len and
token not in kw['ignore_list']])

`