Skip to content

[MRG] insert versionadded versionchanged directives in docstrings for 0.18#5856

Closed
welch wants to merge 2 commits intoscikit-learn:masterfrom
welch:issue-5505-doc-versionadded-0.18
Closed

[MRG] insert versionadded versionchanged directives in docstrings for 0.18#5856
welch wants to merge 2 commits intoscikit-learn:masterfrom
welch:issue-5505-doc-versionadded-0.18

Conversation

@welch
Copy link
Copy Markdown
Contributor

@welch welch commented Nov 16, 2015

issue #5505 versionadded/versionchanged directives for new stuff in 0.18.

It was not clear to me if I should also add these at the file/module level for new files (I did).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is ever rendered / interpreted. It doesn't hurt either, though.

@amueller
Copy link
Copy Markdown
Member

thank you, this is very helpful. The main comment is that for new files, each public class or function in the file needs a versionadded (or versionchanged if it was moved, as the ones in the model_selection module)

@welch
Copy link
Copy Markdown
Contributor Author

welch commented Nov 17, 2015

comments addressed:

  • wording for various min_samples_* versionchanged -> Added float values for percentages.
  • versionadded moved into classes and functions for new files
  • versionchanged "moved from ..." for reorganized files in model_selection/

I wouldn't ordinarily squash mid-review, but it's tidier reading.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All the exceptions where actually all moved, but from different places...

@amueller
Copy link
Copy Markdown
Member

Thanks for all the changes. This looks good. It might be nice to say where the exceptions came from, but I don't feel strongly about it.

@welch
Copy link
Copy Markdown
Contributor Author

welch commented Nov 20, 2015

chased down the original exception class locations and noted in versionadded directives

@amueller
Copy link
Copy Markdown
Member

merged via #7403

@amueller amueller closed this Oct 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants