8000 incremental mean and var for weighted sparse X by maikia · Pull Request #18569 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

incremental mean and var for weighted sparse X #18569

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 92 commits into from
Oct 23, 2020
Merged

incremental mean and var for weighted sparse X #18569

merged 92 commits into from
Oct 23, 2020
+312 −88

Conversation

maikia
Copy link
Contributor
@maikia maikia commented Oct 8, 2020

towards #15601

It adds a possibility to calculate the mean and variance for the sparse X with sample_weight

@maikia
Copy link
Contributor Author
maikia commented Oct 8, 2020

I have some question before continuing @ogrisel and @agramfort

  • is this the right place to put it? or should it be in sklearn.utils.extmath.py ?

  • do we really need axis (now only used for passing if sample_weight is None)? X is normally standard, no?

  • Is this sufficient or shall we write some parts in cython?

@ogrisel ogrisel self-requested a review October 9, 2020 14:38
@agramfort
Copy link
Member
agramfort commented Oct 14, 2020 via email

@scikit-learn scikit-learn deleted a comment from maikia Oct 21, 2020
@agramfort
Copy link
Member

@glemaitre @ogrisel this one should be good to go too.

glemaitre
glemaitre reviewed Oct 22, 2020
View reviewed changes
ogrisel
ogrisel approved these changes Oct 23, 2020
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Mainly style nitpicks (that I will commit) and a request to also tests for dtype=np.float32 + a question on array allocation dtype.

Beside these, LGTM!

@ogrisel ogrisel merged commit f1111be into scikit-learn:master Oct 23, 2020
@ogrisel
Copy link
Member
ogrisel commented Oct 23, 2020

Merged! Thank you very much @maikia and @agramfort. Let's now leverage this in StandardScaler (in a new PR).

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Oct 28, 2020
@maikia @agramfort
@ogrisel @thomasjpfan
incremental mean and var for weighted sparse X (scikit-learn#18569)
7f8bb96 

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
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

@agramfort agramfort agramfort left review comments

@glemaitre glemaitre glemaitre left review comments

@ogrisel ogrisel ogrisel approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted module:utils
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maikia @agramfort @ogrisel @glemaitre @cmarmo
0