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.

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

Fix race condition bug deleting categories #40

Merged
merged 8 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby-version: ['2.7', '3.0', '3.1', '3.2', 'head']
ruby-version: ['2.7', 'head']

steps:
- uses: actions/checkout@v4
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ gemspec

gem 'fast-stemmer'
gem 'matrix'
gem 'mutex_m'
7 changes: 5 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
PATH
remote: .
specs:
classifier (1.4.0)
fast-stemmer (~> 1.0.0)
classifier (1.4.2)
fast-stemmer (~> 1.0)
mutex_m (~> 0.2)
rake

GEM
Expand All @@ -11,6 +12,7 @@ GEM
fast-stemmer (1.0.2)
matrix (0.4.2)
minitest (5.18.1)
mutex_m (0.2.0)
psych (5.1.2)
stringio
rake (13.0.6)
Expand All @@ -28,6 +30,7 @@ DEPENDENCIES
fast-stemmer
matrix
minitest
mutex_m
rdoc

BUNDLED WITH
Expand Down
5 changes: 3 additions & 2 deletions classifier.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = 'classifier'
s.version = '1.4.1'
s.version = '1.4.2'
s.summary = 'A general classifier module to allow Bayesian and other types of classifications.'
s.description = 'A general classifier module to allow Bayesian and other types of classifications.'
s.author = 'Lucas Carlson'
Expand All @@ -9,7 +9,8 @@ Gem::Specification.new do |s|
s.files = Dir['{lib}/**/*.rb', 'bin/*', 'LICENSE', '*.md', 'test/*']
s.license = 'LGPL'

s.add_dependency 'fast-stemmer', '~> 1.0.0'
s.add_dependency 'fast-stemmer', '~> 1.0'
s.add_dependency 'mutex_m', '~> 0.2'
s.add_dependency 'rake'
s.add_development_dependency 'minitest'
s.add_development_dependency 'rdoc'
Expand Down
3 changes: 2 additions & 1 deletion lib/classifier/bayes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,11 @@ def remove_category(category)
category = category.prepare_category_name
raise StandardError, "No such category: #{category}" unless @categories.key?(category)

@total_words -= @category_word_count[category].to_i

@categories.delete(category)
@category_counts.delete(category)
@category_word_count.delete(category)
@total_words -= @category_word_count[category].to_i
end
end
end
36 changes: 30 additions & 6 deletions test/bayes/bayesian_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ def test_remove_category
assert_equal ['Interesting'], @classifier.categories
end

def test_remove_nonexistent_category
assert_raises(StandardError) do
@classifier.remove_category 'NonexistentCategory'
end
end

def test_remove_category_affects_classification
@classifier.train_interesting 'This is interesting content'
@classifier.train_uninteresting 'This is uninteresting content'
Expand Down Expand Up @@ -94,4 +88,34 @@ def test_remove_category_preserves_other_category_data

assert_equal interesting_classification, @classifier.classify('This is interesting')
end

def test_remove_category_check_counts
initial_total_words = @classifier.instance_variable_get(:@total_words)
category_word_count = @classifier.instance_variable_get(:@category_word_count)['Interesting']

@classifier.remove_category('Interesting')

assert_nil @classifier.instance_variable_get(:@categories)['Interesting']
assert_equal @classifier.instance_variable_get(:@category_counts)['Interesting'], 0
assert_equal @classifier.instance_variable_get(:@category_word_count)['Interesting'], 0

new_total_words = @classifier.instance_variable_get(:@total_words)
assert_equal initial_total_words - category_word_count, new_total_words
end

def test_remove_category_updates_total_words_before_deletion
initial_total_words = @classifier.instance_variable_get(:@total_words)
category_word_count = @classifier.instance_variable_get(:@category_word_count)['Interesting']

@classifier.remove_category('Interesting')

new_total_words = @classifier.instance_variable_get(:@total_words)
assert_equal initial_total_words - category_word_count, new_total_words
end

def test_remove_nonexistent_category
assert_raises(StandardError, 'No such category: Nonexistent Category') do
@classifier.remove_category('Nonexistent Category')
end
end
end
Loading