-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
MNT fix section order of what's new v0.24 #17437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
doc/whats_new/v0.24.rst
Outdated
- |Enhancement| Add `sample_weight` parameter to | ||
:class:`metrics.median_absolute_error`. | ||
:pr:`17225` by :user:`Lucy Liu <lucyleeow>`. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
doc/whats_new/v0.24.rst
Outdated
:mod:`sklearn.feature_selection` | ||
................................ | ||
|
There was a problem hiding this comment.
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.
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 |
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) |
since it's already open and reviewed we might as well just merge it ;) |
thanks @lucyleeow |
Reference Issues/PRs
What does this implement/fix? Explain your changes.
Put
metrics
elements under same headerPut
feature_selection
element under correct headerAny other comments?