Support for token processor. Fixes #1156 by wrichert · Pull Request #1537 · scikit-learn/scikit-learn

wrichert · 2013-01-08T08:13:25Z

No description provided.

wrichert · 2013-01-08T08:16:49Z

In the current implementation, CountVectorizer keeps at least one copy per preprocessing step (preprocessing, tokenizing, etc.).

I think we could rewrite it using generators only, which would be more memory and performance friendly.

amueller · 2013-01-08T08:31:09Z

Hi @wrichert. Thanks for the PR. In general I think it looks good.
The test is in the wrong place, though.
The tests for the feature_extaction module are in features_extraction/tests.

wrichert · 2013-01-08T08:41:13Z

Right. Will move it when I have again access to the repo.

wrichert · 2013-01-09T07:38:43Z

Done.

wrichert · 2013-01-10T11:27:34Z

@amueller Is there anything needed from my side to get this PR into the next release?

amueller · 2013-01-10T13:44:14Z

@wrichert Sorry I have not that much time to review right now. Will try tonight. Maybe @ogrisel wants to have a look.

amueller · 2013-01-10T13:44:44Z

Maybe a short example in form of a doctest in the narrative or docstring would be nice?

wrichert · 2013-01-16T16:32:12Z

@amueller Any chance that this is included in 0.13?

amueller · 2013-01-16T16:44:57Z

If you find two devs to review and merge it. I won't have time, sorry.

amueller · 2013-01-26T18:48:10Z

I'll try to have a look soon. Sorry, I'm still pretty busy atm.

ogrisel · 2013-02-07T14:40:51Z

sklearn/feature_extraction/text.py

Cosmetics: please add a blank line above this line (PEP 257).

ogrisel · 2013-02-07T14:43:15Z

Could you please rebase this branch on top of master (or merge master into it if you are not familiar with rebasing).

Then fix pep8 issues reported by: http://pypi.python.org/pypi/pep8

wrichert · 2013-02-08T10:57:16Z

Sure. Will address your suggestions beginning of next week as I won't have time before that.

[MRG] Precompute X_argsorted in AdaBoost

Accidentally committed in 1967a0b.

…transformers. Then fix all the regressors and transformers ... meh!

Prevents warning from doctest.

…learn into token-processor Conflicts: doc/modules/feature_extraction.rst sklearn/feature_extraction/text.py

wrichert · 2013-02-14T07:24:28Z

Hmm, I rebased the "Support for token processor. Fixes #1537" and resolved all the conflicts, but I'm not sure whether I did the right thing, seeing lots of other commits appearing in this threads.

ogrisel · 2013-02-28T13:47:20Z

Indeed please, start a new branch off the current master and cherry pick just the commits or files that are related to issue #1156 to make the review easier.

wrichert added 2 commits January 8, 2013 03:20

Support for token processor. Fixes #1156

9cf5a81

moved test to proper place

6319516

wrichert added 2 commits January 14, 2013 01:31

Added example for token_processor to the docs

5211bc8

Clarifying description of vectorizers' preprocessors.

f759070

ogrisel reviewed Feb 7, 2013
View reviewed changes

sklearn/feature_extraction/text.py Outdated

Copy link
Copy Markdown

Member

ogrisel Feb 7, 2013

Choose a reason for hiding this comment

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

Cosmetics: please add a blank line above this line (PEP 257).

ogrisel and others added 12 commits February 9, 2013 18:08

P3K use six to have a python 2 & 3 compatible code base

1967a0b

P3K StringIO vs BytesIO

0fe516e

DOC fix failing doctest due to unicode_literals

1c0d422

Cosmit: more explicit xlabel.

b445537

Cosmit: more explicit label.

135636e

Merge pull request #1668 from glouppe/adaboost-tree

b126421

[MRG] Precompute X_argsorted in AdaBoost

DOC whitespace in doctest

4c60052

BUG revert P3K changes that broke mldata tests

c809368

rm gender classification example

041abe3

Accidentally committed in 1967a0b.

ENH get rid of imports in test_common by checking by names, not classes.

b99dbab

ENH fix test_estimators_overwrite_params to also test regressors and …

a3b55f4

…transformers. Then fix all the regressors and transformers ... meh!

ENH set the random state to avoid heisenfailures

6920295

amueller and others added 13 commits February 11, 2013 19:18

COSMIT pep8, removing unused imports

7d57b9e

P3K death to the print statement

85ec0fd

P3K fix broken doctest and add forgotten print_function import

c481ff8

DOC no more need for compute_importances in trees

cc401c9

Prevents warning from doctest.

DOC copyedit FeatureHasher narrative

9f6c8bb

ENH move covtype loading to sklearn.datasets

16e595a

TST covertype loader

81428f7

Allowing to drop tokens in token_processor(); pep8 work

44bc5ee

Support for token processor. Fixes #1156

bb5c56e

Added example for token_processor to the docs

e7e9697

Clarifying description of vectorizers' preprocessors.

db35143

Reapplying skipped commit

f5a595a

Merge branch 'token-processor' of https://github.com/wrichert/scikit-…

fceb1a7

…learn into token-processor Conflicts: doc/modules/feature_extraction.rst sklearn/feature_extraction/text.py

ogrisel mentioned this pull request Feb 28, 2013

Add stemming support to CountVectorizer #1156

Closed

wrichert mentioned this pull request Mar 5, 2013

Cleaned and updated version of token-processor support #1735

Closed

wrichert closed this Mar 5, 2013

Uh oh!

Conversation

wrichert commented Jan 8, 2013

Uh oh!

wrichert commented Jan 8, 2013

Uh oh!

amueller commented Jan 8, 2013

Uh oh!

wrichert commented Jan 8, 2013

Uh oh!

wrichert commented Jan 9, 2013

Uh oh!

wrichert commented Jan 10, 2013

Uh oh!

amueller commented Jan 10, 2013

Uh oh!

amueller commented Jan 10, 2013

Uh oh!

wrichert commented Jan 16, 2013

Uh oh!

amueller commented Jan 16, 2013

Uh oh!

amueller commented Jan 26, 2013

Uh oh!

ogrisel Feb 7, 2013

Choose a reason for hiding this comment

Uh oh!

ogrisel commented Feb 7, 2013

Uh oh!

wrichert commented Feb 8, 2013

Uh oh!

wrichert commented Feb 14, 2013

Uh oh!

ogrisel commented Feb 28, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants