Skip to content

MNT fix section order of what's new v0.24#17437

Merged
NicolasHug merged 2 commits intoscikit-learn:masterfrom
lucyleeow:whatsnew
Jun 3, 2020
Merged

MNT fix section order of what's new v0.24#17437
NicolasHug merged 2 commits intoscikit-learn:masterfrom
lucyleeow:whatsnew

Conversation

@lucyleeow
Copy link
Copy Markdown
Member

@lucyleeow lucyleeow commented Jun 3, 2020

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Put metrics elements under same header
Put feature_selection element under correct header

Any other comments?

Comment on lines +71 to +73
- |Enhancement| Add `sample_weight` parameter to
:class:`metrics.median_absolute_error`.
:pr:`17225` by :user:`Lucy Liu <lucyleeow>`.
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.

Space above this and use up more of the 80 characters?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Couldn't do <2 lines without going over 80 characters.

Comment on lines +84 to +86
:mod:`sklearn.feature_selection`
................................

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.

This section (and its contents) should be higher up so that the modules are in alphabetical order.

@NicolasHug
Copy link
Copy Markdown
Member

Happy to merge this when comments are addressed, but FYI we usually do these kind of fixes just before the release. Other discrepancies will come up and we'd rather fix them once

@lucyleeow
Copy link
Copy Markdown
Member Author

lucyleeow commented Jun 3, 2020

Thanks @NicolasHug - I might just delete the PR then, what do you think?

(I noticed this on a merge conflict and it just annoyed me)

@NicolasHug
Copy link
Copy Markdown
Member

since it's already open and reviewed we might as well just merge it ;)

@NicolasHug NicolasHug changed the title DOC Correct header in v0.24.rst MNT fix section order of what's new v0.24 Jun 3, 2020
@NicolasHug NicolasHug merged commit 8cce5bf into scikit-learn:master Jun 3, 2020
@NicolasHug
Copy link
Copy Markdown
Member

thanks @lucyleeow

@lucyleeow lucyleeow deleted the whatsnew branch June 3, 2020 20:00
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 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.

4 participants