Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't train or untrain empty word hashes #132

Merged
merged 4 commits into from
Jan 17, 2017
Merged

don't train or untrain empty word hashes #132

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e297606
don't train or untrain empty word hashes
Ch4s3 Jan 17, 2017
e66c712
first pass on checking word hash first
Ch4s3 Jan 17, 2017
5a4eb3b
simplified classifications and fixed test
Ch4s3 Jan 17, 2017
3c4ff8f
apply test patch
Ch4s3 Jan 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/classifier-reborn/bayes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ def train(category, text)

@backend.update_category_training_count(category, 1)
@backend.update_total_trainings(1)
Hasher.word_hash(text, @language, @enable_stemmer).each do |word, count|
word_hash = Hasher.word_hash(text, @language, @enable_stemmer)
return if word_hash.length == 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make it the very first thing to happen in the method. This current change is essentially changing nothing significantly. update_category_training_count and update_total_trainings are still being called, hence causing undesired stats changes. Also, if the @auto_categorize is set to true, it might add a new category without any trainings in it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to preempt add_category or just update_category_training_count and update_total_trainings?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make it the very first thing in the method, even before calling CategoryNamer. This way, if the training has to be skipped, it does not make any changes in the data-structure state in any circumstances.

word_hash.each do |word, count|
@backend.update_category_word_frequency(category, word, count)
@backend.update_category_word_count(category, count)
@backend.update_total_words(count)
Expand All @@ -91,7 +93,9 @@ def untrain(category, text)
category = CategoryNamer.prepare_name(category)
@backend.update_category_training_count(category, -1)
@backend.update_total_trainings(-1)
Hasher.word_hash(text, @language, @enable_stemmer).each do |word, count|
word_hash = Hasher.word_hash(text, @language, @enable_stemmer)
return if word_hash.length == 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this up in the method as well.

word_hash.each do |word, count|
next if @backend.total_words < 0
orig = @backend.category_word_frequency(category, word) || 0
@backend.update_category_word_frequency(category, word, -count)
Expand Down
2 changes: 1 addition & 1 deletion lib/classifier-reborn/extensions/hasher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def word_hash(str, language = 'en', enable_stemmer = true)

# Return a word hash without extra punctuation or short symbols, just stemmed words
def clean_word_hash(str, language = 'en', enable_stemmer = true)
word_hash_for_words str.gsub(/[^\p{WORD}\s]/, '').downcase.split, language, enable_stemmer
word_hash_for_words(str.gsub(/[^\p{WORD}\s]/, '').downcase.split, language, enable_stemmer)
end

def word_hash_for_words(words, language = 'en', enable_stemmer = true)
Expand Down