8000 [MRG +1] DOC Modify documentation/examples for the new model_selection module by raghavrv · Pull Request #5566 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG +1] DOC Modify documentation/examples for 8000 the new model_selection module #5566

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
Oct 30, 2015
Merged

[MRG +1] DOC Modify documentation/examples for the new model_selection module #5566

merged 1 commit into from
Oct 30, 2015

Conversation

raghavrv
Copy link
Member

Documentation update as a sequel to #4294

@amueller

@fabianp
Copy link 8000
Member
fabianp commented Oct 26, 2015

+1 Looks good to me (also, I'd like to have this before adding documentation of #5491)

@fabianp fabianp changed the title [MRG] DOC Modify documentation/examples for the new model_selection module [MRG +1] DOC Modify documentation/examples for the new model_selection module Oct 26, 2015
@fabianp
Copy link
Member
fabianp commented Oct 26, 2015

Please update with respect to master. You are having travis failures because of fontsize not being supported by matplotlib which I think was solved in master.

@raghavrv
Copy link
Member Author

Done! Thanks for the +1... lets see if travis passes now!!

@Sandy4321
Copy link

for weak learners for ensemble learning you can add individual learners output analysis.
for example all weak learners give similar prediction so overall prediction is good
8000

arjoly
arjoly reviewed Oct 27, 2015
View reviewed changes
doc/modules/classes.rst
:toctree: generated/
:template: function.rst

model_selection.train_test_split
Copy link
Member

Choose a reason for hiding this comment

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

I would put this in the section above.

Copy link
Member Author

Choose a reason for hiding this comment

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

This one has a function template and that one is a class template... So I renamed the headings to "splitter classes" and "splitter functions" I hope that is fine?

@raghavrv
Copy link
Member Author

@arjoly Have addressed your comments :)
@fabianp Could you help me with the doc failure? I am unable to reproduce it using my virtual env setup for python 2.6 + the given versions of nympy and scipy

@raghavrv
Copy link
Member Author

@ogrisel wondering if we need to support doc tests in python 2.6 at all? ;) (BTW this error seems like a job lib related issue... Any suggestions?)

@fabianp
Copy link
Member
fabianp commented Oct 29, 2015

I think the errors that you are experiencing have been fixed in adf866c . If you update again with master I think it should be fixed :-)

@fabianp
Copy link
Member
fabianp commented Oct 29, 2015

Also, I think you should add an entry in doc/whats_new.rst .

@raghavrv
Copy link
Member Author

Hummm this branch has that fix (I've rebased it anyway)... This failure is owing to a non terminating multiprocessing routine in one of the examples of tutorial/statistical_inference/model_selection.rst (which example misbehaves is not clear)...

Also this could mean we have regressed(?) as far as Python 2.6 is concerned as currently all the examples seem to pass (at master as they use the old gs/cv/lc modules) Nope we haven't 😎

@raghavrv
Copy link
Member Author

I have a feeling it has something to do with #5115 (not sure)

@raghavrv
Copy link
Member Author

Ah this seems to fix it... @amueller @fabianp @arjoly please let me know if this fix seems okay and whether we can squash the last 3 commits and have this merged? (Question: Do we need a regression test to make sure this kind of failure does not happen or since #5115 is open, we can leave as such?)

@raghavrv
Copy link
Member Author

EDIT: added a smoke test which failed under python 2.6 as expected.. I'm adding that as a comment to #5115 and removing the commits and squashing them to make it ready for merge!

@raghavrv
Copy link
Member Author

@glouppe If you are online... final +1 and merge?

@fabianp
Copy link
Member
fabianp commented Oct 29, 2015

great, thanks for taking care of it @rvraghav93

@amueller
Copy link
Member

Merging without appveyor as there is no way it will ever catch up.

amueller added a commit that referenced this pull request Oct 30, 2015
@amueller
Merge pull request #5566 from rvraghav93/mod_sel_doc
99530a3 
[MRG +1] DOC Modify documentation/examples for the new model_selection module
@amueller amueller merged commit 99530a3 into scikit-learn:master Oct 30, 2015
@raghavrv
Copy link
Member Author

Yay!! 🍻

@raghavrv raghavrv deleted the mod_sel_doc branch October 30, 2015 21:05
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.
5 participants
@raghavrv @fabianp @Sandy4321 @amueller @arjoly
0