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: Improve description of array scalar in glossary #18965

Merged
merged 2 commits into from
May 13, 2021

Conversation

TheCheshireCat6
Copy link
Contributor

Distinction of array scalars and 0-dimension arrays. See issue #17744.

Co-authored-by: Jasmin Classen 39028086+jasmincl@users.noreply.github.com

Distinction of array scalars and 0-dimension arrays. See issue numpy#17744.

Co-authored-by: Jasmin Classen <39028086+jasmincl@users.noreply.github.com>
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @TheCheshireCat6 and @jasmincl! Overall this description looks good, there's just a small textual issue and a rendering issue:

image

You can see the result of the doc build after CI has run by clicking on "Details" here:

image

For uniformity in handling operands, NumPy treats
a :doc:`scalar <reference/arrays.scalars>` as an array of zero
dimension.
An :doc:` array scalar <reference/arrays.scalars>` is an instance of the types/classes float32, float64,
Copy link
Member

Choose a reason for hiding this comment

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

The space after doc: will be the cause of the rendering issue.

Copy link
Member

Choose a reason for hiding this comment

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

And the same 2 lines below

An :doc:` array scalar <reference/arrays.scalars>` is an instance of the types/classes float32, float64,
etc.. For uniformity in handling operands, NumPy treats a scalar as
an array of zero dimension. In contrast, a 0-dimensional array is an :doc:` ndarray <reference/arrays.ndarray>` instance
containing precisely one array scalar.
Copy link
Member

Choose a reason for hiding this comment

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

The extended description is great, until "containing precisely one array scalar" which seems imprecise. An ndarray doesn't contain an array scalar - there's no instance of a dtype type inside the array container, just a number. How about rephrasing that to "containing precisely one value"?

@rgommers
Copy link
Member

One other thing: if you include Closes gh-17744 in the commit message of your next commit, then the issue will be auto-closed when your PR gets merged.

@charris charris changed the title DOC: Adjusting description of array scalar in glossary DOC: Improve description of array scalar in glossary May 12, 2021
@jasmincl
Copy link
Contributor

One other thing: if you include Closes gh-17744 in the commit message of your next commit, then the issue will be auto-closed when your PR gets merged.

Thanks for the quick feedback! We did not mention "closes gh-17744" because we want to add examples of code to .shape and .ndim with 0-dim ndarrays since we did not have time on Sunday.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Looks good now, merging. Thanks @TheCheshireCat6 and @jasmincl!

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

3 participants