[MRG+1] check length of feature_names in export_graphviz (scikit-learn#8477) (scikit-learn#8512)

* scikit-learn#8477

check length of feature_names in export_graphviz

- raise ValueError if len(feature_names) > tree.n_features
- add unit test for len(feature_names) > tree.n_features
- change the comment of existing unit test for len(feature_names) <
tree.n_features

* fix error and warning

- include length of feature_names and number of features in tree in the
error and warning message.
- raise error for too few feature_names
- for too much feature_names, will use the first n_features. raise an
warning for users
- use assert_raise_message and assert_warns_message in test to check
message.

* move the error and warning from node_to_str to export_graphviz so it will fail early for wrong length of feature_names

* raise error if length of feature_names does not match number of features in the decision tree

* fix pep8

* remove unused assert_warns_message import in test_export.py

* add bug fix in doc/whats_new.rst

* fix the english in doc/whats_new.rst

* fix the format and english in sklearn/tree/export.py

* fix contributor format in doc/whats_new.rst

* fix english, use bracket and avoid \ in error message

* fix pep8

* fix pep8

Author: aikinogard

doc/whats_new.rst

@@ -276,6 +276,11 @@ Bug fixes
    - Fixed a bug in :class:`svm.OneClassSVM` where it returned floats instead of
      integer classes. :issue:`8676` by :user:`Vathsala Achar <VathsalaAchar>`.
 
+   - Fixed a bug where :func:`sklearn.tree.export_graphviz` raised an error
+     when the length of features_names does not match n_features in the decision
+     tree.
+     :issue:`8512` by :user:`Li Li <aikinogard>`.
+
 API changes summary
 -------------------
 

sklearn/tree/export.py

@@ -8,6 +8,7 @@
 #          Noel Dawe <noel@dawe.me>
 #          Satrajit Gosh <satrajit.ghosh@gmail.com>
 #          Trevor Stephens <trev.stephens@gmail.com>
+#          Li Li <aiki.nogard@gmail.com>
 # License: BSD 3 clause
 
 import numpy as np
@@ -172,7 +173,8 @@ def get_color(value):
             if len(sorted_values) == 1:
                 alpha = 0
             else:
-                alpha = int(np.round(255 * (sorted_values[0] - sorted_values[1]) /
+                alpha = int(np.round(255 * (sorted_values[0] -
+                                            sorted_values[1]) /
                                            (1 - sorted_values[1]), 0))
         else:
             # Regression tree or multi-output
@@ -330,7 +332,8 @@ def recurse(tree, node_id, criterion, parent=None, depth=0):
                         # Find max and min impurities for multi-output
                         colors['bounds'] = (np.min(-tree.impurity),
                                             np.max(-tree.impurity))
-                    elif tree.n_classes[0] == 1 and len(np.unique(tree.value)) != 1:
+                    elif (tree.n_classes[0] == 1 and
+                          len(np.unique(tree.value)) != 1):
                         # Find max and min values in leaf nodes for regression
                         colors['bounds'] = (np.min(tree.value),
                                             np.max(tree.value))
@@ -399,6 +402,16 @@ def recurse(tree, node_id, criterion, parent=None, depth=0):
             return_string = True
             out_file = six.StringIO()
 
+        # Check length of feature_names before getting into the tree node
+        # Raise error if length of feature_names does not match
+        # n_features_ in the decision_tree
+        if feature_names is not None:
+            if len(feature_names) != decision_tree.n_features_:
+                raise ValueError("Length of feature_names, %d "
+                                 "does not match number of features, %d"
+                                 % (len(feature_names),
+                                    decision_tree.n_features_))
+
         # The depth of each node for plotting with 'leaf' option
         ranks = {'leaves': []}
         # The colors to render each node with

sklearn/tree/tests/test_export.py

@@ -9,6 +9,7 @@
 from sklearn.tree import export_graphviz
 from sklearn.externals.six import StringIO
 from sklearn.utils.testing import assert_in, assert_equal, assert_raises
+from sklearn.utils.testing import assert_raise_message
 from sklearn.exceptions import NotFittedError
 
 # toy sample
@@ -218,9 +219,17 @@ def test_graphviz_errors():
 
     clf.fit(X, y)
 
-    # Check feature_names error
-    out = StringIO()
-    assert_raises(IndexError, export_graphviz, clf, out, feature_names=[])
+    # Check if it errors when length of feature_names
+    # mismatches with number of features
+    message = ("Length of feature_names, "
+               "1 does not match number of features, 2")
+    assert_raise_message(ValueError, message, export_graphviz, clf, None,
+                         feature_names=["a"])
+
+    message = ("Length of feature_names, "
+               "3 does not match number of features, 2")
+    assert_raise_message(ValueError, message, export_graphviz, clf, None,
+                         feature_names=["a", "b", "c"])
 
     # Check class_names error
     out = StringIO()