8000 test cases for feature_extraction.text by rlmv · Pull Request #1660 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

test cases for feature_extraction.text #1660

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

Merged
merged 11 commits into from
Feb 7, 2013
Merged

Conversation

rlmv
Copy link
Contributor
@rlmv rlmv commented Feb 6, 2013

Extends test coverage in feature_extraction.text and fixes a few typos in the module's documentation.


# error with bad analyzer
vm.analyzer = 'invalid_analyzer'
assert_raises(ValueError, vm.build_analyzer)
Copy link
Member

Choose a reason for hiding this comment

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

I would rather have those tests for the CountVectorizer / HashingVectorizer classes instead as their are less subject to refactoring (public API) while VectorizerMixin is more private API hence more subject to future change.

@ogrisel
Copy link
Member
ogrisel commented Feb 7, 2013

Thanks. I will merge this as soon as you address the previous comment. If you don't have time / will to address it, I can also merge anyway as this is already a net benefit.

@rlmv
Copy link
Contributor Author
rlmv commented Feb 7, 2013

Nah, no problem - I should be able to get to it later today.

@rlmv
Copy link
Contributor Author
rlmv commented Feb 7, 2013

That should be it. I moved the tests for _check_stop_lists to run from a CountVectorizer as well.

ogrisel added a commit that referenced this pull request Feb 7, 2013
test cases for feature_extraction.text
@ogrisel ogrisel merged commit 0e97a7f into scikit-learn:master Feb 7, 2013
@ogrisel
Copy link
Member
ogrisel commented Feb 7, 2013

Thanks!

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.

2 participants
0