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.

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

DOC: Wrong return type of np.random.choice and wrong variable name in parameter description. #8962

Merged
merged 1 commit into from
Apr 26, 2017
Merged

Conversation

MSeifert04
Copy link
Contributor

This fixes some minor documentation issues in np.random.choice:

  • The first parameter is called a and not n as indicated by the np.arange(n) comparison.
  • The return value can be a scalar, a 1D array or an ND-array, currently the documentation suggests that it always returns a 1D array and the shape (possibly the size as well?).

@@ -1040,7 +1040,7 @@ cdef class RandomState:

Returns
--------
samples : 1-D ndarray, shape (size,)
samples : int or ndarray
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int isn't really correct here - it's either an arbitrary scalar (size == None), or an ndarray

@@ -1026,7 +1026,7 @@ cdef class RandomState:
-----------
a : 1-D array-like or int
If an ndarray, a random sample is generated from its elements.
If an int, the random sample is generated as if a was np.arange(n)
If an int, the random sample is generated as if a was np.arange(a)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"As if a were np.arange(a)". Backticks might be nice here too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I omitted adding backticks because most of random documentation does not include them. And it's kind of nicer to read without code-formatting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that at least np.arange should be backticked, but I don't care too much

@MSeifert04
Copy link
Contributor Author

Is there something that needs changing or is it just waiting for another review?

@eric-wieser eric-wieser merged commit f63e37d into numpy:master Apr 26, 2017
@eric-wieser
Copy link
Member

Thanks Michael!

@MSeifert04 MSeifert04 deleted the random_choice_docs branch April 26, 2017 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants