Skip to content
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

Move Statistics out of the sysimage #45594

Merged
merged 1 commit into from
Jun 8, 2022
Merged

Move Statistics out of the sysimage #45594

merged 1 commit into from
Jun 8, 2022

Conversation

nalimilan
Copy link
Member

And remove uses of it in the manual and tests.

Follow-up from #44247. Should probably run PkgEval if the PR looks OK.

I'd suggest trying this, and only move mean and/or std to Base if people complain too loudly before 1.9 is released. Personally I would rather keep these in Statistics to be group statistics function in a single package at last (merging with StatsBase). This makes sense e.g. because these have methods taking weight vectors that we have considered turning into a keyword argument. Also std relies on var so these cannot really be separated.

And remove uses of it in the manual and tests.
@nalimilan nalimilan added stdlib Julia's standard library domain:statistics The Statistics stdlib module labels Jun 6, 2022
@ViralBShah
Copy link
Member

The main argument is that mean and std are used commonly by people as general tools and people who are not doing serious statistics. I personally am ok with moving these, since it is easier to install packages than ever before.

@nalimilan
Copy link
Member Author

@nanosoldier runtests(ALL, vs = ":master")

@ViralBShah
Copy link
Member

This is just moving out of the sysimage, so should be practically risk free.

@JuliaLang JuliaLang deleted a comment from nalimilan Jun 6, 2022
@ViralBShah ViralBShah added the kind:excision Removal of code from Base or the repository label Jun 6, 2022
@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@nalimilan
Copy link
Member Author

I don't see anything suspicious in the 23 new package test failures reported by Nanosoldier.

@ViralBShah
Copy link
Member

We should merge once @KristofferC gives a green light.

Copy link
Sponsor Member

@KristofferC KristofferC left a comment

Choose a reason for hiding this comment

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

I think this should be fine. Fully moving it out requires a little bit of thinking about how to deal with compat but I have a strategy that I think should work.

@nalimilan nalimilan merged commit 54b92a7 into master Jun 8, 2022
@nalimilan nalimilan deleted the nl/Statistics branch June 8, 2022 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:statistics The Statistics stdlib module kind:excision Removal of code from Base or the repository stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants