Upgrade to rand 0.7.0

[max-sixty]

Would close #658

I'm not sure what the policy is on upgrading dependencies; posting as draft

[max-sixty]

Does anyone have an immediate idea of how to resolve this:

error[E0277]: the trait bound `rand::distributions::Normal: rand::distributions::Distribution<f64>` is not satisfied
  --> tests/accuracy.rs:55:5
   |
55 |     Array::random(d, F32(Normal::new(0., 1.)))
   |     ^^^^^^^^^^^^^ the trait `rand::distributions::Distribution<f64>` is not implemented for `rand::distributions::Normal`
   |
   = note: required because of the requirements on the impl of `rand::distributions::Distribution<f32>` for `ndarray_rand::F32<rand::distributions::Normal>`
   = note: required by `ndarray_rand::RandomExt::random`

error[E0277]: the trait bound `rand::distributions::Normal: rand::distributions::Distribution<_>` is not satisfied
  --> tests/accuracy.rs:60:5
   |
60 |     Array::random(d, Normal::new(0., 1.))
   |     ^^^^^^^^^^^^^ the trait `rand::distributions::Distribution<_>` is not implemented for `rand::distributions::Normal`
   |
   = note: required by `ndarray_rand::RandomExt::random`

error: aborting due to 2 previous errors

[bluss]

Hey - nice to see the work going on here. I approve of the developments :) ndarray-rand has been really simple, it can be version bumped and released whenever either of rand or ndarray gains a version, so it's running at double speed, and it has worked, if you want to continue using that. That said, the active maintainers are the boss.

The error is probably due to mixing types/traits from rand and rand_disr - when ndarray-rand is upgraded, it should no longer use the rand version of Distribution and Normal?

[max-sixty]

Thanks for responding @bluss

I moved the Distribution & Normal over to rand_distr (I think!), but I still get the same errors:

error[E0277]: the trait bound `std::result::Result<rand_distr::Normal<{float}>, rand_distr::NormalError>: rand_distr::Distribution<f64>` is not satisfied
  --> tests/accuracy.rs:56:5
   |
56 |     Array::random(d, F32(Normal::new(0., 1.)))
   |     ^^^^^^^^^^^^^ the trait `rand_distr::Distribution<f64>` is not implemented for `std::result::Result<rand_distr::Normal<{float}>, rand_distr::NormalError>`
   |
   = note: required because of the requirements on the impl of `rand_distr::Distribution<f32>` for `ndarray_rand::F32<std::result::Result<rand_distr::Normal<{float}>, rand_distr::NormalError>>`
   = note: required by `ndarray_rand::RandomExt::random`
error[E0277]: the trait bound `std::result::Result<rand_distr::Normal<{float}>, rand_distr::NormalError>: rand_distr::Distribution<_>` is not satisfied
  --> tests/accuracy.rs:61:5
   |
61 |     Array::random(d, Normal::new(0., 1.))
   |     ^^^^^^^^^^^^^ the trait `rand_distr::Distribution<_>` is not implemented for `std::result::Result<rand_distr::Normal<{float}>, rand_distr::NormalError>`
   |
   = note: required by `ndarray_rand::RandomExt::random`
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0277`.
error: Could not compile `numeric-tests`.

[max-sixty]

Ah - it's obvious actually - Normal::new now returns a Result. So this should work now

[max-sixty]

I bumped the ndarray-rand version to 0.10.0

[max-sixty]

Because of https://github.com/rust-random/getrandom, we need to bump the rust version to 1.32 for this to work. Not sure whether that's acceptable or not?

[LukeMathWalker]

To avoid the issues reported in #658, I believe we should re-export rand's distributions in ndarray-rand.
That was probably the underlying issue in #658 (as someone more cunning than me made me notice this afternoon in an unrelated chat): cargo was correctly setting and fetching the version of rand that ndarray-rand dependend upon, but then a user tries to use it with distributions that are imported from the top-level rand dependency which, for various reasons, could be a different version - hence the incompatibility.

As @bluss said, we are already bumping up ndarray-rand's version in lockstep with both ndarray and rand, so I don't think we should be worried of re-exporting the distributions.
What are your thoughts @max-sixty?

[max-sixty]

I read up on this just a couple of days ago, so my understanding is a bit shaky. I had thought that if a crate exported anything from a dependency, cargo prevented conflicts (but maybe only on cargo publish?. (Assuming nothing is exported, cargo can use multiple versions of a dependency).

What would that suggestion look like? If you have any links to hand for more context, I'm happy to add it to this PR.

[max-sixty]

I think I had read this, which is only a proposal: https://github.com/rust-lang/rfcs/blob/master/text/1977-public-private-dependencies.md

[max-sixty]

And then according to this, rust already prevents dependency conflicts: https://stephencoakley.com/2019/04/24/how-rust-solved-dependency-hell

...but that seems to not be true - I independently tested @ngoldbaum's example, and cargo will let you install rand=0.7.0 alongside the existing version of this crate and then raise an error

[LukeMathWalker]

What would that suggestion look like? If you have any links to hand for more context, I'm happy to add it to this PR.

Sure, let me open a PR against the master branch of ndarray-rand, so that we can also use it to test if what I believe should happen actually does 😛

[jturner314]

This generally looks good to me. I just had comments about the dependency on rand_distr.

Since users seem to keep having confusion with rand versions, we should consider adding a prominent note in the docs that says something like:

ndarray-rand depends on rand 0.7. If you use any other items from rand, you need to specify a compatible version of rand in your Cargo.toml. If you want to use a RNG or distribution from another crate with ndarray-rand, you need to make sure that crate also depends on the correct version of rand. Otherwise, the compiler will return errors saying that the items are not compatible (e.g. that a type doesn't implement a necessary trait).

[max-sixty]

Great, thanks @jturner314

I added the docs note to the readme - is that the right place? Any additional feedback?

[jturner314]

It would be good to also add the note to the main ndarray-rand crate docs for people reading the documentation. (In other words, add it to the //! comment near the top of ndarray-rand/src/lib.rs. You can check what it looks like with cargo doc --open.)

Everything else looks good to me.

[max-sixty]

Ah great, thanks for the pointer @jturner314 - added.

Lmk any other changes

[max-sixty]

If I run cargo fix in ndarray/ndarray-rand I get some errors from outside this repo:

    Checking rand_distr v0.2.1
    Checking ndarray-rand v0.10.0 (/Users/maximilian/workspace/ndarray/ndarray-rand)
error[E0432]: unresolved import `rand::distributions::hidden_export`
  --> /Users/maximilian/.cargo/registry/src/github.com-1ecc6299db9ec823/rand_distr-0.2.1/src/utils.rs:13:26
   |
13 | use rand::distributions::hidden_export::IntoFloat;
   |                          ^^^^^^^^^^^^^ could not find `hidden_export` in `distributions`

error[E0603]: module `weighted` is private
  --> /Users/maximilian/.cargo/registry/src/github.com-1ecc6299db9ec823/rand_distr-0.2.1/src/lib.rs:64:70
   |
64 |     Alphanumeric, Uniform, OpenClosed01, Open01, Bernoulli, uniform, weighted};
   |                                                                      ^^^^^^^^

error[E0599]: no method named `into_float_with_exponent` found for type `u64` in the current scope
   --> /Users/maximilian/.cargo/registry/src/github.com-1ecc6299db9ec823/rand_distr-0.2.1/src/utils.rs:212:26
    |
212 |             (bits >> 12).into_float_with_exponent(1) - 3.0
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^

error[E0599]: no method named `into_float_with_exponent` found for type `u64` in the current scope
   --> /Users/maximilian/.cargo/registry/src/github.com-1ecc6299db9ec823/rand_distr-0.2.1/src/utils.rs:215:26
    |
215 |             (bits >> 12).into_float_with_exponent(0)
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0432, E0599, E0603.
For more information about an error, try `rustc --explain E0432`.
error: Could not compile `rand_distr`.

I don't get the same errors if I close the rand crate and run cargo fix from within rand_distr. Is this anything we need to be concerned about?

[jturner314]

If I run cargo fix in ndarray/ndarray-rand I get some errors from outside this repo

I just tried this and got the same errors until I ran cargo update. It turns out to be an issue in rand_distr's dependency specification of rand. (rand_distr specifies that it is compatible with rand 0.5 and 0.6 when it actually isn't.) I've created rust-random/rand#847 to fix this.

[jturner314]

Everything looks good to me.

[max-sixty]

Any follow-ups here remaining?

[bluss]

I don't know why this is languishing. We should not version bump rand in ndarray-rand without making a release.

[max-sixty]

Was this on me? Apologies if so

[LukeMathWalker]

Mostly on me - I was waiting to understand our decision on #663 to cut a release.

[bluss]

Sorry for overreacting, I had hoped I could fix it by releasing but since the rand distr PR is open it makes sense to resolve it first.