Skip to content

[MRG] Modify description of 'X' in k_means_.py fit methods. #7776

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

Closed
wants to merge 1 commit into from
Closed

[MRG] Modify description of 'X' in k_means_.py fit methods. #7776

wants to merge 1 commit into from

Conversation

kdexd
Copy link

@kdexd kdexd commented Oct 28, 2016

Reference Issue

Fixes #7772

What does this implement/fix? Explain your changes.

It modifies the description of existing description of 'X' in MiniBatchKMeans.fit method, and for the sake of consistence add a similar description in KMeans.fit method.

Any other comments?

There were discussions on the issue about using the term "coordinates" in reference to treat the training sample as a data point. I referred to documentation of similar use cases in R and Julia packages and did somewhat "best of both worlds" thing here. I'm available for amendments in the description right away !

References:

  1. Julia Package Docs: http://clusteringjl.readthedocs.io/en/latest/kmeans.html#kmeans
  2. R Package Docs: https://stat.ethz.ch/R-manual/R-devel/library/stats/html/kmeans.html
  3. OpenCV docs: http://docs.opencv.org/2.4/modules/core/doc/clustering.html

@@ -874,6 +874,9 @@ def fit(self, X, y=None):
Parameters
----------
X : array-like or sparse matrix, shape=(n_samples, n_features)
Numerical matrix of training samples to cluster. Each row
will represent a single training sample, a point in space of
'n_features' dimensions.
Copy link
Author

Choose a reason for hiding this comment

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

This is the first draft of description. Commenting here so that this can be seen directly in PR conversation even after this diff gets outdated by any modifications.

@raghavrv
Copy link
Member

Hi, thanks for the PR. As you noted there seems to be a PR already at #7775.

Your description and survey of docstrings on other packages is very helpful especially for a bigger issue like #3791. Could you close this and instead work on that please?

@kdexd
Copy link
Author

kdexd commented Oct 28, 2016

@raghavrv Sure, why not ! I have used scikit-learn for quite a while, mostly in a bunch of competitions. I'll be glad to reciprocate by making it better by my piece of work 😄 I'll close this PR in favor of #7775.

@kdexd kdexd closed this Oct 28, 2016
@kdexd kdexd deleted the kmeans-fit-doc-fix-7772 branch October 28, 2016 16:07
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.

DOC Description of X missing in KMeans.fit
2 participants