Skip to content

DOC improve random_state docstring random modue#15576

Merged
glemaitre merged 4 commits intoscikit-learn:masterfrom
happilyeverafter95:document_random_state
Jan 9, 2020
Merged

DOC improve random_state docstring random modue#15576
glemaitre merged 4 commits intoscikit-learn:masterfrom
happilyeverafter95:document_random_state

Conversation

@happilyeverafter95
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Addresses part of #15222

What does this implement/fix? Explain your changes.

This PR changes the documentation for random_choice_csc by documenting which parts of the algorithm was randomized.

Any other comments?

@jnothman
Copy link
Copy Markdown
Member

jnothman commented Nov 10, 2019 via email

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Jan 8, 2020

@jnothman, @NicolasHug , I think that @happilyeverafter95 modified this file picking it from the list available in #15222. This list is automatically generated and I'm cleaning it for the next sprints. If you all agree that this file should not be touched I will remove it from the new list, please let us know. Anyway thanks Mandy for your work! And sorry for our late reaction.

@NicolasHug
Copy link
Copy Markdown
Member

i haven't checked the correctness of the comment, but I don't see why we shouldn't also update private utils like this one

@glemaitre
Copy link
Copy Markdown
Member

I agree with both points raised by @jnothman and @NicolasHug. Let's focus on public estimators first.

@glemaitre
Copy link
Copy Markdown
Member

@happilyeverafter95 Feel free to pick up any other docstring from the list (which is an estimator).
Sorry to not have been more reactive.

@glemaitre glemaitre closed this Jan 8, 2020
@NicolasHug
Copy link
Copy Markdown
Member

Sorry if my comment wasn't clear, but I was suggesting that the PR is IMO relevant and the changes should be considered

@glemaitre
Copy link
Copy Markdown
Member

Oh no, it is me that did not read correctly your comment. Sorry about that. Let's reopen and merge

@glemaitre glemaitre reopened this Jan 9, 2020
Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

This can be merged when CIs are green.

@glemaitre glemaitre changed the title Documentation for random_state in random.py DOC improve random_state docstring random modue Jan 9, 2020
@glemaitre glemaitre merged commit eb12449 into scikit-learn:master Jan 9, 2020
keyianpai added a commit to keyianpai/scikit-learn that referenced this pull request Jan 11, 2020
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 22, 2020
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020
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.

5 participants