Repository: scikit-learn/scikit-learn
Pull request: [MRG+1] Added estimator checks for pandas object
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/master/CONTRIBUTING.md#pull-request-checklist
-->

#### Reference Issues/PRs
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->
Fixes #7528 

#### What does this implement/fix? Explain your changes.
Added estimator checks for pandas object.

#### Any other comments?


<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
Add dataframe test to common tests.
We should test that estimators play nice with DataFrames.
Currently there is a test for NotAnArray. We should also explicitly test pandas, I think, with a lazy import.
We now have pandas on one of the CI builds.

Hints from issue discussion:
I am taking this up.

@amueller Could you tell me the files associated with the issue?

utils/estimator_checks.py
testing/test_common.py

Sorry for the delay, it is an AFK, kinda week. I will try my best to patch it up ASAP.

Should I create a NotADataframe class in the estimate_check file, then a check_transformer for the same?

You should be testing whether `X2 = pandas.DataFrame(X)` works the same as `X` for some array `X` in various contexts.

@sidgupta234 Are you still working on this or I can take this ?

Sure, you take this :)
On Oct 9, 2016 9:42 PM, "Aashil" notifications@github.com wrote:

> @sidgupta234 https://github.com/sidgupta234 Are you still working on
> this or I can take this ?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> https://github.com/scikit-learn/scikit-learn/issues/7528#issuecomment-252495452,
> or mute the thread
> https://github.com/notifications/unsubscribe-auth/AIcxTYw1ZY_2ILJ2-rcEEj8pxBRmWOm3ks5qyRJ8gaJpZM4KJ_uU
> .

I am working in this.
@mc4229  do you want to work on this together?

Commit message: Revert "used pytest.importorskip"

This reverts commit 4526d5b07dce43010c31854ab1b62c3b4290c023.

Patch to review:
diff --git a/sklearn/utils/estimator_checks.py b/sklearn/utils/estimator_checks.py
index 6b411fca2a2a4..4f4b6c2ea86e3 100644
--- a/sklearn/utils/estimator_checks.py
+++ b/sklearn/utils/estimator_checks.py
@@ -117,8 +117,8 @@ def _yield_checks(name, estimator):
 def _yield_classifier_checks(name, classifier):
     tags = _safe_tags(classifier)
 
-    # test classifiers can handle non-array data
-    yield check_classifier_data_not_an_array
+    # test classifiers can handle non-array data and pandas objects
+    yield check_classifier_two_data_types
     # test classifiers trained on a single label always return this label
     yield check_classifiers_one_label
     yield check_classifiers_classes
@@ -169,7 +169,7 @@ def _yield_regressor_checks(name, regressor):
     # basic testing
     yield check_regressors_train
     yield partial(check_regressors_train, readonly_memmap=True)
-    yield check_regressor_data_not_an_array
+    yield check_regressor_two_data_types
     yield check_estimators_partial_fit_n_features
     yield check_regressors_no_decision_function
     if not tags["no_validation"]:
@@ -222,7 +222,7 @@ def _yield_outliers_checks(name, estimator):
         yield check_outliers_train
         yield partial(check_outliers_train, readonly_memmap=True)
         # test outlier detectors can handle non-array data
-        yield check_classifier_data_not_an_array
+        yield check_classifier_two_data_types
         # test if NotFittedError is raised
         yield check_estimators_unfitted
 
@@ -2061,24 +2061,26 @@ def check_sparsify_coefficients(name, estimator_orig):
 
 
 @ignore_warnings(category=DeprecationWarning)
-def check_classifier_data_not_an_array(name, estimator_orig):
+def check_classifier_two_data_types(name, estimator_orig):
     X = np.array([[3, 0], [0, 1], [0, 2], [1, 1], [1, 2], [2, 1]])
     X = pairwise_estimator_convert_X(X, estimator_orig)
     y = [1, 1, 1, 2, 2, 2]
     y = multioutput_estimator_convert_y_2d(estimator_orig, y)
-    check_estimators_data_not_an_array(name, estimator_orig, X, y)
+    for obj_type in ["NotAnArray", "PandasDataframe"]:
+        check_estimators_two_data_types(name, estimator_orig, X, y, obj_type)
 
 
 @ignore_warnings(category=DeprecationWarning)
-def check_regressor_data_not_an_array(name, estimator_orig):
+def check_regressor_two_data_types(name, estimator_orig):
     X, y = _boston_subset(n_samples=50)
     X = pairwise_estimator_convert_X(X, estimator_orig)
     y = multioutput_estimator_convert_y_2d(estimator_orig, y)
-    check_estimators_data_not_an_array(name, estimator_orig, X, y)
+    for obj_type in ["NotAnArray", "PandasDataframe"]:
+        check_estimators_two_data_types(name, estimator_orig, X, y, obj_type)
 
 
 @ignore_warnings(category=(DeprecationWarning, FutureWarning))
-def check_estimators_data_not_an_array(name, estimator_orig, X, y):
+def check_estimators_two_data_types(name, estimator_orig, X, y, obj_type):
     if name in CROSS_DECOMPOSITION:
         raise SkipTest("Skipping check_estimators_data_not_an_array "
                        "for cross decomposition module as estimators "
@@ -2089,8 +2091,25 @@ def check_estimators_data_not_an_array(name, estimator_orig, X, y):
     set_random_state(estimator_1)
     set_random_state(estimator_2)
 
-    y_ = NotAnArray(np.asarray(y))
-    X_ = NotAnArray(np.asarray(X))
+    if obj_type not in ["NotAnArray", 'PandasDataframe']:
+        raise ValueError("Data type {0} not supported".format(obj_type))
+
+    if obj_type == "NotAnArray":
+        y_ = NotAnArray(np.asarray(y))
+        X_ = NotAnArray(np.asarray(X))
+    else:
+        try:
+            import pandas as pd
+            y_ = np.asarray(y)
+            if y_.ndim == 1:
+                y_ = pd.Series(y_)
+            else:
+                y_ = pd.DataFrame(y_)
+            X_ = pd.DataFrame(np.asarray(X))
+
+        except ImportError:
+            raise SkipTest("pandas is not installed: not checking estimators "
+                           "for pandas objects.")
 
     # fit
     estimator_1.fit(X_, y_)


Code review comments:
diff_hunk: @@ -117,8 +117,8 @@ def _yield_checks(name, estimator):
 def _yield_classifier_checks(name, classifier):
     tags = _safe_tags(classifier)
 
-    # test classifiers can handle non-array data
-    yield check_classifier_data_not_an_array
+    # test classifiers can handle non-array data and pandas objects
+    yield check_classifier_two_data_types
path: sklearn/utils/estimator_checks.py
text: @user1:
two_data_types is not clear (especially as dtype = data type is something else entirely). not_an_array is still acceptable.