I've done a Python script to find frequent keywords from raw texts and display them along with source sentences and texts. But, having not exercised my Python for an age, I'm sure the code is full of smell. Any pointers would be warmly appreciated.

import os
import nltk
import string
import json
from datetime import datetime
from nltk.tokenize import word_tokenize
from nltk.corpus import stopwords

class Word:
    """Stores data relating to individual words."""

    def __init__(self, spelling):
        self.spelling = spelling
        self.sentences = []
        self.frequency = 0
        self.found_in = []

    def add_sentences(self, *sentences):
        [self.sentences.append(s) for s in sentences]

    def to_json(self):
        structure = {self.spelling: {"Sentences": self.sentences, "Documents": self.found_in}}
        return structure

class Hashtagger:
    """Extracts frequent words from text, and the sentences in which they are found."""

    def __init__(self, raw_text):
        self.raw_text = raw_text
        self.words = []
        self.sentences = []
        self.stopword_list = stopwords.words('english') # words to not process
        self.lemmatizer = nltk.WordNetLemmatizer() # groups inflections to single word

    def process_text(self):

    def process_words(self, text):
        stripped_text = self.strip_punctuation(text)
        tokens = nltk.Text(word_tokenize(stripped_text))
        content = [self.lemmatizer.lemmatize(t.lower()) for t in tokens if t not in self.stopword_list]
        counts = {t: content.count(t) for t in content}

        for token, count in counts.items():
            word = Word(token)
            word.frequency = count

    def deconstruct_sentences(self, text):
        raw_sentences = nltk.sent_tokenize(text)
        self.sentences = [s.strip(string.punctuation) + '.' for s in raw_sentences]

    def allocate_sentences_to(self, word):
        sentences_containing = [s for s in self.sentences if word.spelling in s.lower()]

    def allocate_sentences_to_all(self):
        [self.allocate_sentences_to(key) for key in self.words]

    def strip_punctuation(self, text):
        for ch in string.punctuation:
            text = text.replace(ch, ' ')
        return text

class ResultsAccumulator:
    """Accumulates the results of many hashtaggers."""

    def __init__(self):
        self.word_tally = []

    def add_tally(self, hashtagger, file=None):
        for h_word in hashtagger.words:
            matched = False
            for word in self.word_tally:
                if word.spelling == h_word.spelling:
                    word.frequency += h_word.frequency
                    matched = True
            if not matched:

    def write_word_json(self):
        total_json = {}
        for word in self.word_tally:
        return total_json

def main():
    path = r'test_docs/'

    results = ResultsAccumulator()

    for file in os.listdir(path):
        with open(os.path.join(path, file), encoding='utf8') as d:
            raw_text = d.read()

        hashtagger = Hashtagger(raw_text)
        results.add_tally(hashtagger, file)

    final_tally = results.write_word_json()

    datestring = datetime.strftime(datetime.now(), '%Y-%m-%d-%H-%M-%S')
    with open('word_json_' + datestring + '.txt', 'w') as f:
        json.dump(final_tally, f, indent=4,

if __name__ == '__main__':

From what I can tell, the "weirdest" thing in your code is the following:

Avoid list comprehensions if all you want are side-effects

In your Word class you have the following:

def add_sentences(self, *sentences):
    [self.sentences.append(s) for s in sentences]

That is a weird function because generally list comprehensions are created because we care about the list they actually build, not because we want side-effects. Therefore, if you wanted to create a loop to do the .append you do, you would write something like

def add_sentences(self, *sentences):
    for s in sentences:

Having said that, your sentences is an iterable so what you can do is

def add_sentences(self, *sentences):

*args is nice but not always necessary

Now, if we keep dissecting this function, we see that it is often called with a single argument, which is an iterable (usually a list), so while the *args pattern is nice, it is useless if everything you are going to do is call the function with


The *args syntax is useful for when you don't know how many arguments you are getting and it makes sense to call the function with many arguments. For your use case, I don't think we will see things like

the = Word("the")
the.add_sentences("The cat is nice", "I love the Python language")

This means that you can very well define your code to be

def add_sentences(self, sentences):

And later you call it with


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.