Skip to content

[MRG] Fix description of SVC intercept_ shape in user guide.#12070

Merged
rth merged 1 commit intoscikit-learn:masterfrom
zdgriffith:svc-intercept-shape-fix
Sep 16, 2018
Merged

[MRG] Fix description of SVC intercept_ shape in user guide.#12070
rth merged 1 commit intoscikit-learn:masterfrom
zdgriffith:svc-intercept-shape-fix

Conversation

@zdgriffith
Copy link
Copy Markdown
Contributor

The shape of the intercept_ attribute of sklearn.svm.SVC is correctly [n_class * (n_class-1) / 2] in the docstring but in the user guide it is implied the shape is [n_class]. This PR fixes the description in the user guide.

@zdgriffith zdgriffith changed the title Fix description of SVC intercept_ shape in user guide. [MRG] Fix description of SVC intercept_ shape in user guide. Sep 14, 2018
Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

It would also be good to emphasize in the docsting for SVC.coef_ that the shape is not necessarily [n_class-1, n_features].

Now it uses several levels of indirection:
coef_ -> read only of dual_coef_ -> The layout of the coefficients in the multiclass case is somewhat non-trivial, see docs.

@zdgriffith
Copy link
Copy Markdown
Contributor Author

Ah, my mistake, I linked to the stable version of the docstring. It looks like that was addressed by PR #11660, the dev page has the correct coef_ shape.

@jnothman
Copy link
Copy Markdown
Member

I suspect I'm missing something with respect to @rth's comment... Did you amend a commit @zdgriffith ? If so, please avoid it.

@zdgriffith
Copy link
Copy Markdown
Contributor Author

No, in the initial Github comment where I said "..in the docstring" I linked to v0.19.2 of the docs. @rth's suggestion regarding SVC.coef_ was valid there but has since been fixed by the pull request I linked to in my last comment. Hope that clears it up?

Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

I must indeed have checked the outdated docs..

Thanks for this PR!

@rth rth merged commit abbdc91 into scikit-learn:master Sep 16, 2018
jnothman pushed a commit to jnothman/scikit-learn that referenced this pull request Sep 17, 2018
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.

3 participants