Skip to content

[MRG +1] Fix text data tutorial #6923

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 23, 2016
Merged

[MRG +1] Fix text data tutorial #6923

merged 1 commit into from
Jun 23, 2016

Conversation

crodjer
Copy link
Contributor

@crodjer crodjer commented Jun 22, 2016 •
edited
Loading

Reference Issue

Getting a doctest error in my local:

$ nosetests doc/tutorial/text_analytics/working_with_text_data.rst 
/home/rohan/workspace/src/scikit-learn/doc/tutorial/text_analytics/working_with_text_data.rst:1: VisibleDeprecationWarning: converting an array with ndim > 0 to an index will result in an error in the future
  .. _text_data_tutorial:
F
======================================================================
FAIL: Doctest: working_with_text_data.rst
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python3.5/doctest.py", line 2189, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for working_with_text_data.rst
  File "/home/rohan/workspace/src/scikit-learn/doc/tutorial/text_analytics/working_with_text_data.rst", line 0

----------------------------------------------------------------------
File "/home/rohan/workspace/src/scikit-learn/doc/tutorial/text_analytics/working_with_text_data.rst", line 452, in working_with_text_data.rst
Failed example:
    gs_clf.best_score_
Expected:
    0.900...
Got:
    0.90000000000000002

>>  raise self.failureException(self.format_failure(<_io.StringIO object at 0x7f8f5f7ed798>.getvalue()))


----------------------------------------------------------------------
Ran 1 test in 8.932s

FAILED (failures=1)

What does this implement/fix? Explain your changes.

Use doctest: +ELLIPSIS to fix the issue and fix a type in the document.

Any other comments?

None

@crodjer
Copy link
Contributor Author

crodjer commented Jun 22, 2016

Also, after the fix, I get the following VisibleDepricationWarning:

/home/rohan/workspace/src/scikit-learn/doc/tutorial/text_analytics/working_with_text_data.rst:1: VisibleDeprecationWarning: converting an array with ndim > 0 to an index will result in an error in the future
  .. _text_data_tutorial:
.
----------------------------------------------------------------------
Ran 1 test in 8.712s

OK

Is this expected behaviour? I couldn't find the origin of this.

@amueller
Copy link
Member

LGTM. Not sure what recent change triggered this.

@amueller amueller changed the title Fix text data tutorial [MRG +1] Fix text data tutorial Jun 22, 2016
@maniteja123
Copy link
Contributor

Hi, I am not sure if it is correct but I suppose the VisibleDeprecationWarning is from this line where the index is an array but not a scalar. AFAIR this should raise a TypeError in numpy master now. Changing the line from

  >>> twenty_train.target_names[gs_clf.predict(['God is love'])]

to

  >>> twenty_train.target_names[gs_clf.predict(['God is love'])[0]]

should fix the warning IMO. Cheers.

jnothman
jnothman reviewed Jun 23, 2016
View reviewed changes
doc/tutorial/text_analytics/working_with_text_data.rst
@@ -449,7 +449,7 @@ that we can use to ``predict``::
The object's ``best_score_`` and ``best_params_`` attributes store the best
mean score and the parameters setting corresponding to that score::

>>> gs_clf.best_score_
>>> gs_clf.best_score_ # doctest: +ELLIPSIS
Copy link
Member

Choose a reason for hiding this comment

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

should be two spaces before #

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jnothman Fixed. Instead of two spaces, I have aligned it with rest of the existing # doctest: +ELLIPSIS declarations.

@jnothman
Copy link
Member

Not sure what recent change triggered this.

#6697 I suspect.

@jnothman jnothman mentioned this pull request Jun 23, 2016
Closed
@jnothman
Copy link
Member

This looks good to me, and I'm happy to merge; let me know if you'd rather add in a fix for the print statement.

@crodjer
Fix text data tutorial
aab0300 
 - Fix a typo.
 - Fix a floating point error in doctests.
 - Fix `VisibleDepricationWarning` due to conversion of an array with
   ndim > 0 to an index.
 - Convert `print` to `log` to avoid doctests failure.

Fixes: scikit-learn#6927

Signed-off-by: Rohan Jain <crodjer@gmail.com>
@crodjer
Copy link
Contributor Author

crodjer commented Jun 23, 2016

Included the fix for twenty_newsgroups. I had to break the line into multiple to follow the 80 character limit. I couldn't find the standard way to do this via a quick grep through the project.

@jnothman
Copy link
Member

Thanks.

@jnothman jnothman merged commit 4a2bc34 into scikit-learn:master Jun 23, 2016
olologin pushed a commit to olologin/scikit-learn that referenced this pull request Aug 24, 2016
@crodjer @olologin
DOC Fix text data tutorial (scikit-learn#6923)
fc88443 
 - Fix a typo.
 - Fix a floating point error in doctests.
 - Fix `VisibleDepricationWarning` due to conversion of an array with
   ndim > 0 to an index.
 - Convert `print` to `log` to avoid doctests failure.

Fixes: scikit-learn#6927

Signed-off-by: Rohan Jain <crodjer@gmail.com>
TomDLT pushed a commit to TomDLT/scikit-learn that referenced this pull request Oct 3, 2016
@crodjer @TomDLT
DOC Fix text data tutorial (scikit-learn#6923)
41c06ea 
 - Fix a typo.
 - Fix a floating point error in doctests.
 - Fix `VisibleDepricationWarning` due to conversion of an array with
   ndim > 0 to an index.
 - Convert `print` to `log` to avoid doctests failure.

Fixes: scikit-learn#6927

Signed-off-by: Rohan Jain <crodjer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@crodjer @amueller @maniteja123 @jnothman