Skip to content
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.

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

Sgd rename #26

Closed
wants to merge 10 commits into from
Closed

Sgd rename #26

wants to merge 10 commits into from

Conversation

pprett
Copy link
Member

@pprett pprett commented Dec 2, 2010

  • Moved sgd examples to examples/linear_model
  • Prefixed examples with sgd
  • Moved covertype example to benchmarks
  • Updated documentation (linear_model.rst, classes.rst)

@ogrisel
Copy link
Member

ogrisel commented Dec 2, 2010

The overall look and feel looks fine to me: +1 for the merge if the test pass and the documentation build with the auto-examples on.

@fabianp
Copy link
Member

fabianp commented Dec 2, 2010

Thanks for this. There are some errors in the import paths of the examples when you import "from scikits.learn.linear_model.stochastic_gradient.sparse" , it should be from "scikits.learn.linear_model.sparse" .

As a suggestion, some other paths can be shortened (which I think its good on examples) : the stochastic_gradient in "from scikits.learn.linear_model.stochastic_gradient import SGDClassifier" is superfluous.

+1 for the merge once the import paths have been fixed.

COSMIT import paths in plot examples.
Rephrased SGD outline in linear_model.rst
@pprett
Copy link
Member Author

pprett commented Dec 3, 2010

thanks for reminding me... I completely forgot to run the non-auto examples. I've now fixed the import paths both in the auto and non-auto examples -> I'll push the changes to master.

thanks,
Peter

@fabianp
Copy link
Member

fabianp commented Dec 3, 2010

Great, thanks. No excuse for me to not do my part now :-)

glouppe pushed a commit to glouppe/scikit-learn that referenced this pull request Feb 1, 2013
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants