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.

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

Implement rand::Rand #12

Closed
termoshtt opened this issue Jan 22, 2018 · 7 comments
Closed

Implement rand::Rand #12

termoshtt opened this issue Jan 22, 2018 · 7 comments
Labels
enhancement help wanted

Comments

@termoshtt
Copy link
Contributor

num-complex::Complex<T> should implement rand::Rand if T: Rand is assured.
@cuviper
Copy link
Member

cuviper commented Jan 22, 2018

As an optional feature, sure!

@vks
Copy link
Contributor

vks commented Jan 25, 2018

Rand is likely to get deprecated.

@cuviper
Copy link
Member

cuviper commented Jan 25, 2018

@vks Do you have more information about that? What's the replacement?

@vks
Copy link
Contributor

vks commented Jan 25, 2018

@cuviper It will be replaced with something similar, see dhardy/rand#83.

@cuviper
Copy link
Member

cuviper commented Jan 25, 2018

Hmm. It seems fine to go ahead and support rand as it exists today, and we can migrate to the new thing later along with everyone else. With a shim crate it's even possible to support both.

@shingtaklam1324 shingtaklam1324 mentioned this issue Feb 13, 2018
Merged
@shingtaklam1324
Copy link
Contributor

@vks If we decide to stick to supporting Rust 1.8 for num-complex, that won't be an issue as rand v0.4+ won't even compile on rustc 1.8.0

bors bot added a commit that referenced this issue Mar 6, 2018
@bors
Merge #14
1eaae98 
14: Implementing rand::Rand r=cuviper a=shingtaklam1324

Implements #12 .

Right now it is implemented with calling `rng.gen::<T>()` for both the real and imaginary parts. I'm not 100% sure on the mathematical randomness of the distribution if the complex number is used as a vector and the angle is wanted.

As well as that, for the dependency on `rand`, I just pulled the newest version on crates.io, but it may be better to use another version. I don't have many projects that use this so I'm not sure how big of an issue this will be.

Changes:

- Optional `rand` feature and dependency on `rand`
- `impl<T> rand::Rand for Complex<T>`
@shingtaklam1324 shingtaklam1324 mentioned this issue Mar 7, 2018
Merged
@cuviper
Copy link
Member

cuviper commented Mar 9, 2018

Done in #14.

@cuviper cuviper closed this as completed Mar 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@vks @cuviper @termoshtt @shingtaklam1324