DOC improve docstring in OPTICS functions compute_optics_graph#22024
Conversation
sklearn/cluster/_optics.py
Outdated
| distance between them. This works for Scipy's metrics, but is less | ||
| efficient than passing the metric name as a string. If metric is | ||
| "precomputed", X is assumed to be a distance matrix and must be square. | ||
| "precomputed", 'X' is assumed to be a distance matrix and must be square. |
There was a problem hiding this comment.
It would be better to use the following.
| "precomputed", 'X' is assumed to be a distance matrix and must be square. | |
| "precomputed", `X` is assumed to be a distance matrix and must be square. |
sklearn/cluster/_optics.py
Outdated
| ---------- | ||
| X : ndarray of shape (n_samples, n_features), or \ | ||
| (n_samples, n_samples) if metric=’precomputed’. | ||
| (n_samples, n_samples) if metric=’precomputed’ |
There was a problem hiding this comment.
These quotes are weird.
| (n_samples, n_samples) if metric=’precomputed’ | |
| (n_samples, n_samples) if metric="precomputed" |
There was a problem hiding this comment.
What do you think if we use instead:
(n_samples, n_samples) if metric='precomputed'
I think it is more consistent with the rest of the code.
|
So this is not a big deal for this PR. But be aware that you did not created a branch from I closed #22024 and we will merge both changes. If you want to do new PR in the future, please |
|
I see that another PR was merged that fixed most of the issues. There is still a couple of nitpicking improvements that I will merge even if they are not required to pass the NumPy doc test. |
|
Thanks @Moonkyung94 |
Reference Issues/PRs
#21350
What does this implement/fix? Explain your changes.
End the sentence with a period.
Any other comments?