[MRG] t-SNE perplexity docstring update#13069
Conversation
sklearn/manifold/t_sne.py
Outdated
| between 5 and 50. The choice is not extremely critical since t-SNE | ||
| is quite insensitive to this parameter. | ||
| between 5 and 50. While the choice is not extremely critical, results | ||
| have been shown to change considerably across perplexity values. |
There was a problem hiding this comment.
That sounds a bit contradictory. Why not simply remove the entire sentence?
There was a problem hiding this comment.
I think it is important for the user to know that results can change considerably across perplexity values. I could either remove the sentence or do something like:
Changing the perplexity value can significantly impact the result.
Either way, I think at the very least the sentence should be removed.
sklearn/manifold/t_sne.py
Outdated
| usually require a larger perplexity. Consider selecting a value | ||
| between 5 and 50. While the choice is not extremely critical, results | ||
| have been shown to change considerably across perplexity values. | ||
| between 5 and 50. Different values can result in significanlty different results. |
There was a problem hiding this comment.
@NicolasHug the one I previously proposed made it seem like it was suggesting that changing the value from the default (30) would impact the results, hence this wording.
There was a problem hiding this comment.
The wording is fine to me, but the checks are failing since the line is too long. Please wrap around 79 characters
|
Thanks @kjacks21 |
This reverts commit 97299ed.
This reverts commit 97299ed.
Reference Issues/PRs
Addresses #13040
What does this implement/fix? Explain your changes.
Updating the perplexity parameter docstring in order to show that the perplexity value does impact the results more than what the current docstring implied.