search

JuliaStats / StatsFuns.jl

Mathematical functions related to statistics.
Other
235 stars 40 forks source link

Reapply #113 and prepare release of StatsFuns 1.0.0 #142

Closed devmotion closed 2 years ago

devmotion commented 2 years ago

This PR is a cleaned and corrected version of #139. Copied from there:

This PR is a follow-up to https://github.com/JuliaStats/StatsFuns.jl/pull/138 and reapplies https://github.com/JuliaStats/StatsFuns.jl/pull/113. It updates the compat entries and tests the oldest supported Julia version + LTS to ensure that users don't end up with (major) regressions compared with StatsFuns 0.9.17 that uses Rmath instead of Julia implementations of these functions.

codecov-commenter commented 2 years ago

Codecov Report

Merging #142 (ff7c85d) into master (a93d6b2) will increase coverage by 7.61%. The diff coverage is 95.55%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
+ Coverage   48.98%   56.60%   +7.61%     
==========================================
  Files          13       13              
  Lines         445      530      +85     
==========================================
+ Hits          218      300      +82     
- Misses        227      230       +3
Impacted Files Coverage Δ
src/distrs/tdist.jl 100.00% <ø> (ø)
src/distrs/binom.jl 78.57% <62.50%> (-21.43%) :arrow_down:
src/distrs/gamma.jl 97.91% <97.61%> (-2.09%) :arrow_down:
src/distrs/beta.jl 100.00% <100.00%> (ø)
src/distrs/chisq.jl 100.00% <100.00%> (ø)
src/distrs/fdist.jl 100.00% <100.00%> (ø)
src/distrs/pois.jl 100.00% <100.00%> (ø)
src/distrs/norm.jl 97.43% <0.00%> (+1.23%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a93d6b2...ff7c85d. Read the comment docs.

devmotion commented 2 years ago

@nalimilan I did not remove the reexports for now since it seems there was no general agreement in favour of this change (at least I remember you preferred them to be kept).

nalimilan commented 2 years ago

@nalimilan I did not remove the reexports for now since it seems there was no general agreement in favour of this change (at least I remember you preferred them to be kept).

Yeah my point was that if we drop IrrationalConstants and LogExpFunctions reexports, this package only contains distributions-related functions, so it would better be renamed e.g. DistributionsFuns. Maybe at some point we could split out these functions to a new package, and StatsFuns would become a meta package and/or be deprecated.

Before releasing 1.0, are we sure we are happy with all of the API? AFAICT it's been very stable so I guess it's fine?

devmotion commented 2 years ago

Before releasing 1.0, are we sure we are happy with all of the API? AFAICT it's been very stable so I guess it's fine?

Yes, in my opinion it has been quite stable, I think the main consideration was just whether to drop the reexports or not.

In any case, we can always move to 2.0 if something comes up that is worth changing but breaking.

andreasnoack commented 2 years ago

In any case, we can always move to 2.0 if something comes up that is worth changing but breaking.

If that is our thinking then there is there is little reason to release 1.0. IMO releasing 1.0 only makes sense if we expect to commit to the API for some time.

devmotion commented 2 years ago

If that is our thinking then there is there is little reason to release 1.0.

Well, that's my general opinion when people mention that some package should be moved to 1.0.

For StatsFuns I think it's not likely that there are any breaking changes anytime soon but I don't think moving to 1.0 should mean that useful breaking changes should be discouraged completely. However, even though I'm generally a bit hesitant regarding these pushes towards 1.0, there are some advantages I think: It is possible to distinguish between bugfix and feature releases, and one can also drop older Julia versions in a non-breaking way while being able to backport fixes to older Julia versions if needed.

nalimilan commented 2 years ago

It doesn't seem there are any problems with the API (and my request on Slack didn't prompt any comments), so I'd say it's fine to release 1.0 -- unless you think we should drop reexports.

devmotion commented 2 years ago

I don't mind either way, I think it's OK to keep them (for now at least). Just a final check as a follow-up to our discussion above, are you happy with the PR @andreasnoack?

andreasnoack commented 2 years ago

Yes. Let's move forward with this as it is.