8000 DOC fix the changelog and group entries together by glemaitre · Pull Request #27421 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC fix the changelog and group entries together #27421

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Sep 20, 2023
Merged

DOC fix the changelog and group entries together #27421

merged 5 commits into from
Sep 20, 2023

Conversation

glemaitre
Copy link
Member

Fixing some issues seen in the changelog of 1.4.
Also grouped together PRs linked to support for sparse matrices.

I also think it could be meaningful to do the same for the Array-API if we think that we will have a rather wide support for the next release. I did not do it here. I would let @ogrisel @betatim to let me know what they think.

@github-actions
Copy link
github-actions bot commented Sep 19, 2023 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 6544338. Link to the linter CI: here

@betatim
Copy link
Member
betatim commented Sep 20, 2023

I think grouping things for Array API support makes sense. Though from a marketing point of view I think we should not call it "Array API support" but something like "GPU support" or something. Mostly this is a communication thing. Most people who work on this stuff know the plan (Array API for python, plugins for cython) but people not directly involved often bring up "yeah but array api won't work ..." because they seem to assume that all we are doing is adding Array API support. (This is mostly a side comment to start y'all brains thinking :D )

adrinjalali
adrinjalali reviewed Sep 20, 2023
View reviewed changes
Copy link
Member
@adrinjalali adrinjalali 8000 left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

@ogrisel
Copy link
Member
ogrisel commented Sep 20, 2023 •
edited
Loading

I also think it could be meaningful to do the same for the Array-API if we think that we will have a rather wide support for the next release. I did not do it here. I would let @ogrisel @betatim to let me know what they think.

+1 for a section named "Array API and GPU support (experimental)".

ogrisel
ogrisel reviewed Sep 20, 2023
View reviewed changes
doc/whats_new/v1.4.rst Outdated
- |Enhancement| :class:`preprocessing.TargetEncoder` now supports `target_type`
'multiclass'. :pr:`26674` by :user:`Lucy Liu <lucyleeow>`.

- |MajorFeature| :class:`preprocessing.MinMaxScaler` and
Copy link
Member

Choose a reason for hiding this comment

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

We should respect the standard label ordering MajorFeature > Feature > ... > Enhancement > Fix.

ogrisel
ogrisel approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM once the ordering of the entries is fixed.

@StefanieSenger StefanieSenger mentioned this pull request Sep 20, 2023
Merged
@ogrisel
Copy link
Member
ogrisel commented Sep 20, 2023

The CI has failed because of a problem in compose.rst unrelated to this PR. Let me trigger a sync with main.

lorentzenchr
lorentzenchr approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member
@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

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

BTW, this is very hard to properly review:smirk:

@glemaitre glemaitre merged commit ba7d869 into scikit-learn:main Sep 20, 2023
lesteve pushed a commit to lesteve/scikit-learn that referenced this pull request Sep 28, 2023
@glemaitre @ogrisel @lesteve
DOC fix the changelog and group entries together (scikit-learn#27421)
ad78235 
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@glemaitre @ogrisel @REDVM
DOC fix the changelog and group entries together (scikit-learn#27421)
0bdeeb9 
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali left review comments

@ogrisel ogrisel ogrisel approved these changes

@lorentzenchr lorentzenchr lorentzenchr approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@glemaitre @betatim @ogrisel @adrinjalali @lorentzenchr
0