10000 DOC: clarify documentation for transpose() by danpovey · Pull Request #15024 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

DOC: clarify documentation for transpose() #15024

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
Merged
merged 3 commits into from
Dec 6, 2019
Merged

DOC: clarify documentation for transpose() #15024

merged 3 commits into from
Dec 6, 2019

Conversation

danpovey
Copy link
Contributor
@danpovey danpovey commented Dec 2, 2019

No description provided.

@danpovey danpovey force-pushed the transpose_doc_patch branch from e49c534 to dedee47 Compare December 2, 2019 02:54
@danpovey danpovey force-pushed the transpose_doc_patch branch from dedee47 to 9bf03f6 Compare December 2, 2019 02:57
@XVilka XVilka mentioned this pull request Dec 2, 2019
Closed
seberg
seberg reviewed Dec 2, 2019
View reviewed changes
numpy/core/fromnumeric.py Outdated
according to the values given.
axes : tuple or list of ints, optional
If not specified, this function reverses the order of the axes.
If specified, it must be a tuple, list or similar object which contains
Copy link
Member

Choose a reason for hiding this comment

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

"similar object", is typically called "sequence" in python language. In fact, tuple or list can probably also be replaced with sequence. Unless we want to nudge people towards tuple specifically (since in many cases axes has to be a tuple).

Looks good to me.

Copy link
Member

Choose a reason for hiding this comment

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

I would lean towards documenting it as just tuple or list, and not encouraging users to to pass anything else.

eric-wieser
eric-wieser reviewed Dec 3, 2019
View reviewed changes
eric-wieser
eric-wieser reviewed Dec 3, 2019
View reviewed changes
eric-wieser
eric-wieser reviewed Dec 3, 2019
View reviewed changes
numpy/core/fromnumeric.py Outdated
Comment on lines 608 to 609
For an array a with two axes, transpose(a) gives the matrix transpose
of a.
Copy link
Member

Choose a reason for hiding this comment

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

I think PEP257 needs a blank line before this sentence.

Suggested change
For an array a with two axes, transpose(a) gives the matrix transpose
of a.
For an array ``a`` with two axes, ``transpose(a)`` gives the matrix transpose
of a.

Copy link
Member

Choose a reason for hiding this comment

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

This comment still applies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed fix

@danpovey
Copy link
Contributor Author
danpovey commented Dec 3, 2019

Addressed the comments

@seberg
Copy link
Member
seberg commented Dec 3, 2019

Thanks, if you still have some patience. I noticed that the return section says it normally returns a view. That is incorrect. transpose will always return a view.

@danpovey
Copy link
Contributor Author
danpovey commented Dec 3, 2019 via email

@eric-wieser
Copy link
Member
eric-wieser commented Dec 3, 2019 •
edited
Loading

That is incorrect. transpose will always return a view.

You wish... transpose will return whatever the subclass of ndarray decides to implement their transpose method as, which in the past has not always been a view. I think this came up when I was implementing #8441.

Channeling past you:

I guess a subclass could cause a copy (the code looks like subclassing doing something fancy is not be anticipated, there may be a bug here),

@danpovey danpovey force-pushed the transpose_doc_patch branch from d56e5a1 to a563892 Compare December 3, 2019 21:11
@danpovey
Copy link
Contributor Author
danpovey commented Dec 3, 2019 via email

@seberg
Copy link
Member
seberg commented Dec 3, 2019

That is incorrect. transpose will always return a view.

You wish... transpose will return whatever the subclass of ndarray decides to implement their transpose method as, which in the past has not always been a view. I think this came up when I was implementing #8441

:(

@eric-wieser
Copy link
Member

@seberg: See my edit - we discussed this before.

@danpovey
Copy link
Contributor Author
danpovey commented Dec 5, 2019

Guys, if I am supposed to make any change to this it's not clear to me. Just a reminder- please clarify if it's not OK.

@seberg
Copy link
Member
seberg commented Dec 6, 2019

Sounds like an improvement. Thanks @danpovey.

@seberg seberg merged commit 95e570c into numpy:master Dec 6, 2019
@seberg seberg mentioned this pull request Dec 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seberg seberg seberg left review comments

@eric-wieser eric-wieser eric-wieser left review comments

Assignees
No one assigned
Labels
04 - Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danpovey @seberg @eric-wieser @mattip
0