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

Document ndindex(shape) #16745

Closed
inducer opened this issue Jul 3, 2020 · 3 comments · Fixed by #16821
Closed

Document ndindex(shape) #16745

inducer opened this issue Jul 3, 2020 · 3 comments · Fixed by #16821

Comments

@inducer
Copy link
Contributor

inducer commented Jul 3, 2020

This usage of ndindex:

>>> list(np.ndindex((3,2)))
[(0, 0), (0, 1), (1, 0), (1, 1), (2, 0), (2, 1)]

seems to work with

>>> np.__version__
'1.18.4'

but it's not documented. Is this usage intended to be allowed? (I'd like it to be, because if shape == 5 then np.index(shape) works while np.ndindex(*shape) does not.) If it is, could it be added to the docs?

@seberg
Copy link
Member

seberg commented Jul 5, 2020

We may want to make that version the preferred documented one, and just briefly mention the other option as well. In most cases (e.g. also arr.reshape() where we allow both, we prefer the tuple-version in the docs).

@rossbar
Copy link
Contributor

rossbar commented Jul 6, 2020

Just to clarify this a bit - I think there are two things that make the np.ndindex docstring confusing:

  1. The signature of the np.ndindex docstring is inconsistent with the Parameters listing
  2. Additionally, the type listing in the Parameters section is incorrect: both tuples and list of ints are allowed.

@rohitsanj
Copy link
Contributor

rohitsanj commented Jul 12, 2020

Additionally, the type listing in the Parameters section is incorrect: both tuples and list of ints are allowed.

This has been fixed.

The signature of the np.ndindex docstring is inconsistent with the Parameters listing

I will be raising a PR for this :)

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

Successfully merging a pull request may close this issue.

4 participants