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

Method to add custom path to user-created stopword directory #73

Merged
merged 2 commits into from Jun 2, 2016
Merged

Method to add custom path to user-created stopword directory #73

merged 2 commits into from Jun 2, 2016

Conversation

chris357
Copy link
Contributor

@chris357 chris357 commented Jun 2, 2016

Per discussion in Functionality to modify stopword lists? thread. I added a method to hasher module so users can add a custom path to a directory with user-made stopword files.

def add_custom_stopword_path(path)
  STOPWORDS_PATH.unshift(path)
end

Not sure it adds a tonne of value over the current way to hack it,
Current:

include ClassifierReborn
Hasher::STOPWORDS_PATH.unshift("/home/ubuntu/workspace/data")

But I think it makes adding a path slightly more intuitive for someone not digging into the code.
Updated:

include ClassifierReborn
Hasher.add_custom_stopword_path("/home/ubuntu/workspace/data")

Thoughts?

@Ch4s3
Copy link
Member

Ch4s3 commented Jun 2, 2016

This looks like a nice simple solution. Can you add a test for the method?

I'm trying to think if this warrants some refactoring of the Hasher module.

@chris357
Copy link
Contributor Author

chris357 commented Jun 2, 2016

I've added a test which creates a tempfile of stopwords, and then adds that tempfile's path using the add_custom_stopword_path method.

  1. Test is a little verbose (mostly because you don't know a tempfile's exact name until after creation). So added lots of comments.
  2. Also needed to change the original require File.dirname(__FILE__) + '/../test_helper' to require_relative '../test_helper' because my IDE didn't like the absolute path.

Let me know your thoughts.

@Ch4s3
Copy link
Member

Ch4s3 commented Jun 2, 2016

I think it looks good. Any refactoring of the hashing function is probably best left for later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants