statrs-dev / statrs

Statistical computation library for Rust
https://docs.rs/statrs/latest/statrs/
MIT License
599 stars 84 forks source link

Make rand optional #275

Closed FreezyLemon closed 2 months ago

FreezyLemon commented 2 months ago

After #270, making rand optional means that it's possible to use a very stripped-down version of the crate:

$ cargo tree -e normal --no-default-features
statrs v0.17.1
├── approx v0.5.1
│   └── num-traits v0.2.19
└── num-traits v0.2.19

Like the nalgebra feature, rand is enabled by default. This added modularity might make #165 easier, too.

This requires a breaking change: Removing the rand::distribution::Distribution<T> supertrait from statrs::distribution::Distribution<T> and ::DiscreteDistribution<T>. Semantically, it means that the statrs distributions are not required to be rand distributions and are not logical extensions of them. Practically, it just means that new distributions don't necessarily have to implement the rand Distribution trait (all current distributions still do implement it with the feature enabled).

I would argue that the distribution traits still make sense without rand/RNG. This also allows using most of the statistics module without needing to compile rand and all that entails.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 9.52381% with 38 lines in your changes missing coverage. Please review.

Project coverage is 93.70%. Comparing base (d0a5b04) to head (5ae4760).

Files with missing lines Patch % Lines
src/distribution/categorical.rs 0.00% 2 Missing :warning:
src/distribution/dirichlet.rs 0.00% 2 Missing :warning:
src/distribution/gamma.rs 0.00% 2 Missing :warning:
src/distribution/multivariate_normal.rs 0.00% 2 Missing :warning:
src/distribution/negative_binomial.rs 0.00% 2 Missing :warning:
src/distribution/normal.rs 0.00% 2 Missing :warning:
src/distribution/poisson.rs 0.00% 2 Missing :warning:
src/distribution/triangular.rs 0.00% 2 Missing :warning:
src/statistics/traits.rs 0.00% 2 Missing :warning:
src/distribution/bernoulli.rs 0.00% 1 Missing :warning:
... and 19 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #275 +/- ## ========================================== + Coverage 93.54% 93.70% +0.16% ========================================== Files 53 53 Lines 11657 11636 -21 ========================================== Hits 10904 10904 + Misses 753 732 -21 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

YeungOnion commented 2 months ago

Thanks again for what you've put in with the last 24 hours. It looks good, just gotta handle the merge.

FreezyLemon commented 2 months ago

This should be good to go. I resolved the merge conflicts manually, and the CI failure is just codecov

FreezyLemon commented 2 months ago

Needed to rebase again after the other PRs were merged.

YeungOnion commented 2 months ago

coverage CI looks stuck. I'm messing around with it on poorly named pr #279

YeungOnion commented 2 months ago

Now, I will merge the correct thing. Whoops.