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

Basics: improve random number examples #255

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@dhardy
Contributor

dhardy commented Jul 22, 2017

I noticed the random examples were quite short and didn't document range for floating point cases. Improved.

Caveat: this doesn't explain why the .ind_sample method is used. I don't understand myself why the Sample trait (taking mutable reference to the range) exists, and if it does have a use-case it should probably have been named MutSample. But I digress.

@budziq

Hi @dhardy,, thanks for taking interest!

Some of our examples (as well as the actual rand crate) certainly need more polish. We'd love to have you on board! 👍

Here are some suggestions below (sorry for the terse form but I'm currently on holidays mobile only)

Show outdated Hide outdated src/basics.md Outdated
Show outdated Hide outdated src/basics.md Outdated
Show outdated Hide outdated src/basics.md Outdated
Show outdated Hide outdated src/basics.md Outdated
@@ -122,11 +124,30 @@ extern crate rand;
use rand::Rng;
fn main() {
// Each thread has an automatically-initialised random number generator:

This comment has been minimized.

@budziq

budziq Jul 24, 2017

Collaborator

generally such information would be conveyed in the textual description mentioning key code identifiers linked to crate docs instead of comments (we try to keep comments to absolute minimum striving to make descriptions and code as self explanatory as possible)

@budziq

budziq Jul 24, 2017

Collaborator

generally such information would be conveyed in the textual description mentioning key code identifiers linked to crate docs instead of comments (we try to keep comments to absolute minimum striving to make descriptions and code as self explanatory as possible)

Show outdated Hide outdated src/basics.md Outdated
Show outdated Hide outdated src/basics.md Outdated
@dhardy

This comment has been minimized.

Show comment
Hide comment
@dhardy

dhardy Jul 24, 2017

Contributor

@budziq updated, including changing the "normal distributions" section to be a "general distributions" section and reference the previous usage.

Contributor

dhardy commented Jul 24, 2017

@budziq updated, including changing the "normal distributions" section to be a "general distributions" section and reference the previous usage.

@budziq

few more suggestions and a commit squash into a single one

| [Generate random numbers within a range][ex-rand-range] | [![rand-badge]][rand] | [![cat-science-badge]][cat-science] |
| [Generate random numbers with normal distribution][ex-rand-dist] | [![rand-badge]][rand] | [![cat-science-badge]][cat-science] |
| [Generate random number distributions][ex-rand-dist] | [![rand-badge]][rand] | [![cat-science-badge]][cat-science] |

This comment has been minimized.

@budziq

budziq Jul 24, 2017

Collaborator

"Generate random number distributions" is slightly inaccurate. We are not generating distributions but rather random number with/of/described by given distribution. I would suggest to rewrite the title as:
"Generate random number with given distribution"

@budziq

budziq Jul 24, 2017

Collaborator

"Generate random number distributions" is slightly inaccurate. We are not generating distributions but rather random number with/of/described by given distribution. I would suggest to rewrite the title as:
"Generate random number with given distribution"

Show outdated Hide outdated src/basics.md Outdated
Creates a [`Normal`] distribution with mean `3` and standard deviation `5`
and generates a random value with [`IndependentSample::ind_sample`].
You've already seen how to create numbers with uniform distribution [above][ex-rand-range]
(using [`Range`]). Other distributions are used in the same way: create a distribution, then

This comment has been minimized.

@budziq

budziq Jul 24, 2017

Collaborator

most of this description is obvious from the trivial code below no need to be verbose here.

@budziq

budziq Jul 24, 2017

Collaborator

most of this description is obvious from the trivial code below no need to be verbose here.

[![rand-badge]][rand] [![cat-science-badge]][cat-science]
Creates a [`Normal`] distribution with mean `3` and standard deviation `5`
and generates a random value with [`IndependentSample::ind_sample`].
You've already seen how to create numbers with uniform distribution [above][ex-rand-range]

This comment has been minimized.

@budziq

budziq Jul 24, 2017

Collaborator

we don't aim to make the cookbook into a tutorial (what I've meant by "progression" was if there is any missing information between examples or if these are not sorted by their difficulty). We strive for problem solution (hence cookbook) approach of self-contained examples.

I would roll back the original description here as it was short and to the point and just link to alternative distributions.

@budziq

budziq Jul 24, 2017

Collaborator

we don't aim to make the cookbook into a tutorial (what I've meant by "progression" was if there is any missing information between examples or if these are not sorted by their difficulty). We strive for problem solution (hence cookbook) approach of self-contained examples.

I would roll back the original description here as it was short and to the point and just link to alternative distributions.

let mut rng = rand::thread_rng();
// mean 2, standard deviation 3:
let normal = Normal::new(2.0, 3.0);

This comment has been minimized.

@budziq

budziq Jul 24, 2017

Collaborator

any reason to change the order of statements or distribution parameters?

@budziq

budziq Jul 24, 2017

Collaborator

any reason to change the order of statements or distribution parameters?

@@ -566,8 +563,11 @@ fn main() {
[`Read`]: https://doc.rust-lang.org/std/io/trait.Read.html
[`Normal`]: https://doc.rust-lang.org/rand/rand/distributions/normal/struct.Normal.html
[`IndependentSample::ind_sample`]: https://doc.rust-lang.org/rand/rand/distributions/trait.IndependentSample.html#tymethod.ind_sample
[`rand`]: https://doc.rust-lang.org/rand

This comment has been minimized.

@budziq

budziq Jul 24, 2017

Collaborator

there is already a link to rand crate with proper form.

[normal link]
`code`
[`linked code`]
@budziq

budziq Jul 24, 2017

Collaborator

there is already a link to rand crate with proper form.

[normal link]
`code`
[`linked code`]
Show outdated Hide outdated src/basics.md Outdated
Show outdated Hide outdated src/basics.md Outdated
Show outdated Hide outdated src/basics.md Outdated
@dhardy

This comment has been minimized.

Show comment
Hide comment
@dhardy

dhardy Jul 24, 2017

Contributor

You know, if you can put this much effort into constructive criticism, I might as well let you write it yourself. There's only so much time I'm prepared to waste on a cookbook I'm sure most people won't give more than a quick glance anyway.

Contributor

dhardy commented Jul 24, 2017

You know, if you can put this much effort into constructive criticism, I might as well let you write it yourself. There's only so much time I'm prepared to waste on a cookbook I'm sure most people won't give more than a quick glance anyway.

@budziq

This comment has been minimized.

Show comment
Hide comment
@budziq

budziq Jul 24, 2017

Collaborator

You know, if you can put this much effort into constructive criticism, I might as well let you write it yourself.

Well it certainly takes a lot of effort and motivation considering I am juggling +10PR's while on vacation with only mobile at my disposal.

Each an every one is held under the same scrutiny in order to have consistent format and quality. Please note that while the presentation and form might not look like it the cookbook is aimed to become an official resource once its out of its pre-alpha stage.

I agree that contribution is painful at the moment (I guess some kind of guideline for the examples would help)

There's only so much time I'm prepared to waste on a cookbook

Well I would not call it "wasting" you are helping a lot. For instance I would certainly fix the examples myself if I had even a slightest notion that there is a problem in them.

We are thinking about the final form and format for cookbook and if anything, your problems and suggestions will have an impact on the final output.

I guess that this PR might go smother If we had discussed your suggestions prior to you making the actual work. But unfortunately I did lack the means to drive the conversation.

Sorry to have discouraged you :(

Collaborator

budziq commented Jul 24, 2017

You know, if you can put this much effort into constructive criticism, I might as well let you write it yourself.

Well it certainly takes a lot of effort and motivation considering I am juggling +10PR's while on vacation with only mobile at my disposal.

Each an every one is held under the same scrutiny in order to have consistent format and quality. Please note that while the presentation and form might not look like it the cookbook is aimed to become an official resource once its out of its pre-alpha stage.

I agree that contribution is painful at the moment (I guess some kind of guideline for the examples would help)

There's only so much time I'm prepared to waste on a cookbook

Well I would not call it "wasting" you are helping a lot. For instance I would certainly fix the examples myself if I had even a slightest notion that there is a problem in them.

We are thinking about the final form and format for cookbook and if anything, your problems and suggestions will have an impact on the final output.

I guess that this PR might go smother If we had discussed your suggestions prior to you making the actual work. But unfortunately I did lack the means to drive the conversation.

Sorry to have discouraged you :(

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Jul 25, 2017

Contributor

This looks great, @dhardy. Thanks. I rebased and merged it. I did change the language in the non-uniform distributions example to not reference previous examples. For the moment at least it is true that the examples are pretty independent, and it's pretty likely that the format of the book will change drastically before any kind of final release.

I do think it would be worth having a better narrative progression between examples so that it reads nicely as a book, leads the reader on a journey of some kind. This is great for now though - we're going to have to circle back for a big editing pass in the future.

Thanks for reviewing @budziq (but do feel free to take some time off while you are traveling).

Contributor

brson commented Jul 25, 2017

This looks great, @dhardy. Thanks. I rebased and merged it. I did change the language in the non-uniform distributions example to not reference previous examples. For the moment at least it is true that the examples are pretty independent, and it's pretty likely that the format of the book will change drastically before any kind of final release.

I do think it would be worth having a better narrative progression between examples so that it reads nicely as a book, leads the reader on a journey of some kind. This is great for now though - we're going to have to circle back for a big editing pass in the future.

Thanks for reviewing @budziq (but do feel free to take some time off while you are traveling).

@brson brson closed this Jul 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment