I think a list of indices is actually easier from a user perspective, since a boolean mask is easily translated to indices with np.where but the other way around takes more work.

Thinking about it, DictVectorizer will probably take a list of feature names so a list of feature indices will be closer to that. I will like the API to be as close as possible in OneHotEncoder and DictVectorizer.

If we go for a list of features indices / names, we need to choose if it's more convenient to list the categorical variables or the non-categorical ones. Which is best highly depends on the application...

For DictVectorizer, I could also use a dict feature-name => True / False.

DictVectorizer does one-hot coding when fed (str, str) pairs, which is not applicable here (right?).

I think it's more convenient to list the categorical ones, because otherwise you'd have a parameter called noncategorical_features, which is more typing :)

Travis has caught a doctest to update: https://travis-ci.org/scikit-learn/scikit-learn/builds/7766345#L2752

DictVectorizer does one-hot coding when fed (str, str) pairs, which is not applicable here (right?).

I thought that it does the one-hot coding even for integer features but I probably misread the code. Then, great, I indeed do not need to change anything.

It looks like DictVectorizer considers something categorical iff its value is a string, and I don't understand how categorical_features would help.

noncategorical_features, which is more typing

but ignore_mask or ignored_features or held_out_features takes less than either, and might apply more broadly to transformers...? I'm not sure about it.

However, I do think this can be pulled out as a generic util: transform_selected(transform, X, selected) := np.hstack(transform(X[:, selected]), X[:, ~selected]) with sparse support, etc...

I thought of ignore_features but it is ambiguous in that one might think that the features are removed. I just leave them as is.

categorical_features is the only obvious choice that I can think of.

Any other opinion on indices vs. mask? Another thing I like about masks is that you can use np.ones or np.zeros to generate the initial mask then fill in the few categorical or non-categorial variables for your application of interest. This way we are not favoring applications with few categorical variables.

Any other opinion on indices vs. mask?

I don't want to complicate things, but wouldn't it be possible to support both? I was thinking about converting (implicitely) masks to indices (or vice-versa depending on what you want to feed to the implementation). If the array is as long as the number of features, then it is a boolean mask and can be converted to indices, otherwise it is assumed to be an array of indices. Couldn't this work?

Indeed, it shouldn't be too hard to support both. Thanks!

That leads to ambiguity. What if n_features=2 and categorical_features=array([0, 1])?

I say we leave it up to the user to call np.where and maybe put that in an example.

@larsmans Indeed, you are right. By the way, what are they doing in Numpy in such a case? (since you can index an array both with a mask or a list of indices)

Numpy seems to rely on the type of the array in that case.

In [2]: a = np.arange(2)

In [5]: a[np.array([0,1], dtype=np.bool)]
Out[5]: array([1])

In [6]: a[np.array([0,1])]
Out[6]: array([0, 1])

Anyway, up to you. I don't want to complicate things for nothing :)

Indeed, if we want to support both, the user has to explicity set dtype=bool in the case of masks. That sounds fair enough to me.

categorical_features is the only obvious choice that I can think of.

Instead of the ignored_features or noncategorical_features one could use passthrough_features to be both explicit and use a positive formulation. But I am also ok with the current categorical_features.

passthrough_features would make it clear if, for instance, the same option were available on a feature selector or PCA.

I added combined support of masks and index arrays. I kept categorical_features which I find to the point and directly related to what OneHotEncoder is doing. If there's no further comment, I will update the user guide tomorrow.

Indirectly related to this PR: does any one know how to handle variables which only make sense if another variable takes some particular value ? For example, a variable which only makes sense if the patient is a female (e.g., number of years since menopause)

I guess I'd handle that as a quadratic feature: female × years_past_menopause. Then delete the original years_past_menopause feature. Or, using DictVectorizer,

female = patient.sex == FEMALE
features['female'] = female
if female:
    features['years_past_menopause'] = patient.menopause  # default of 0 when missing

I'd still suggest this belongs as a utility function, not something special to OneHotEncoder.

enc = OneHotEncoder(categorical_features=cat)
X_train = enc.fit_tansform(X_train)
X_test = enc.transform(X_test)

vs.

enc = OneHotEncoder()
X_train = transform_selected(X_train, enc.fit_transform, categorical_features=cat)
X_test = transform_selected(X_test, enc.transform, categorical_features=cat)

vs.

enc = OneHotEncoder()

X_cat, X_non_cat = split_selected(X_train)
X_cat = enc.fit_transform(X_cat)
X_train = sp.hstack((X_cat, X_non_cat))

X_cat, X_non_cat = split_selected(X_test)
X_cat = enc.transform(X_cat)
X_test = sp.hstack((X_cat, X_non_cat))

Any other opinions? Abstractions are nice but it's also important to have easy-to-use APIs...

If scikit-learn was based on abstractions like this, we wouldn't need OVR support directly in SGDClassifier and would rely on OneVsRestClassifier instead.

I should clarify, because you're getting me completely wrong here and making a farce: I don't think you should require the user to use the abstraction. Of course the user can build such an abstraction out of appropriate transformers, Pipelines and FeatureUnions, but that's absolutely mad.

Still, you can create a helper function and use it only in OneHotEncoder for now. Doing so:

• gives a name to what you're doing rather than nesting the OneHotEncoder code with some obscure _apply_transform to handle an edge-case parameter ("explicit is better than implicit");
• allows you to test its functionality as a unit;
• enables other transformer implementations to adopt the same functionality without having to reinvent the wheel (to handle the boolean vs integer index, the sparse vs dense cases, etc).

I am sorry for not making this clearer in the first place; but surely your interpretation of my suggestion is so patently ridiculous (not to mention even messier if providing a class API with parameters, etc.) that it must be a misinterpretation.

However, I do think this can be pulled out as a generic util: transform_selected(transform, X, selected) := np.hstack(transform(X[:, selected]), X[:, ~selected]) with sparse support, etc...

I was not trying to make a farce of you. From the above comment, I thought you were suggesting to let the user call the utility, not to use it in OneHotEncoder directly. If you get angry so easily you should try to be clearer in the first place.

I will try to implement your suggestion tonight. If I create such an abstraction, while I am it, I can also create some kind of FeatureSplit transformer.

I didn't get angry until I felt my suggestion was being ridiculed.

Unless you can come up with some convincingly simple API, I don't think FeatureSplit is worthwhile (nor is its name especially intuitive). If the user wants such abstracted functionality, it should come by the way of some SelectByIndex(mask_or_indices, negate=False) that acts like other feature selectors, and can indeed be combined with FeatureUnion and Pipeline to create a monster that does exactly what it's told to (and can be controlled by set_params in a fairly logical manner).

Alternatively, you could just add a parameter to FeatureUnion that allows you to mask the input for each transformer (but then one requires an implementation of the IdentityTransformer to get the same expressiveness). I think that's actually what you mean by FeatureSplit.

(And that suggestion of adding a parameter to FeatureUnion is very similar to one I made elsewhere, except that there I did not assume X was a 2d array, but possibly a dict or an object.)

I didn't get angry until I felt my suggestion was being ridiculed.

In case of possible misunderstanding, my view is to always believe that
the person I am discussing with is being nice, and I have not understood
him. I find that at least 50% if the case it is true, so I am definitely
doing as good as chance, but I get double benefit that I feel better, and
the person also.

I think that this suggestion could have been applied both ways here :)

@jnothman I agree the FeatureUnion could be improved to be made more generic to better deal with heterogeneous data (e.g. a sequence of dicts with free text fields, dates, numeric values and string encoded category names for instance). Ideally we would want a simple way to pass some sort of "feature router". Maybe we could discuss strategies to implement this during the sprint in Paris. If so we first need to come up with a set of common use cases (I have a couple in mind).

In the mean time +1 for splitting the OneHotEncoder categorical feature un-masking utility code as a utility function (private for now so as not to have to handle backward compat in case we change our minds on how to best deal with this situation).

I agree the FeatureUnion could be improved to be made more generic to better deal with heterogeneous data

Might as well make an issue so this is remembered: #2034. (And it's looking unlikely that I'll be at the sprint.)

In the mean time +1 for splitting the OneHotEncoder categorical feature un-masking utility code as a utility function (private for now so as not to have to handle backward compat in case we change our minds on how to best deal with this situation).

Done.

Can I haz merge?

Shall we rename categorical_features to just encode? It's shorter, it's imperative, and maybe the user actually wants to encode some categorical variables, but not others.

Otherwise, +1.

Shall we rename categorical_features to just encode? It's shorter, it's imperative, and maybe the user actually wants to encode some categorical variables, but not others.

+1

Very cool. I made a few minor comment (feel free not to address them, even though I feel that some should :).

Besides that, 👍 for merge.

@larsmans I personally prefer the more explicit categorical_features but I am glad to change if more people prefer encode.

@larsmans I personally prefer the more explicit categorical_features but I am
glad to change if more people prefer encode.

I think that I prefer categorical_features, as it is more explicit.

But the statistical term might be confusing to new users. Also, encode is equally explicit :)

We have a stalemate. Anyone for either categorical_features or encode? @glouppe? @jnothman? When that's resolved, I'm up for merging and fixing the little things in master.

Anyone for either categorical_features or encode?

I prefer categorical_features.

Ok, that's 3-2 for you guys. Whoever still objects gets to submit an issue (I won't ;)

Thanks Lars!