Skip to content

Conversation

@ahaldane
Copy link
Member

@ahaldane ahaldane commented May 5, 2017

These are updated structure arrays docs to reflect the changes in #6053.

Don't merge this before #6053.

I'm putting them here now for comments to accompany #6053.

While this PR is open I will maintain an HTML compiled version of these docs at https://ahaldane.github.io/user/basics.rec.html

Copy link
Member

Choose a reason for hiding this comment

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

Not a complaint about this PR, but I don't think I like this behaviour:

>>> np.dtype([('', 'f4'),('f0', 'i4'),('z', 'i8')])
ValueError: field 'f0' occurs more than once

In #9054, I change this in some cases to be "index within the unnamed values". Is that a good thing?

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 it might be clearer to show these as >>> dt = np.dtype(...), >>> np.zeros(1, dtype=dt)

Copy link
Member

Choose a reason for hiding this comment

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

It seems that arrays.dtypes.rst duplicates a lot of the contents here, and perhaps these should be condensed into a single help page

Copy link
Member

Choose a reason for hiding this comment

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

Missing comma

Copy link
Member

Choose a reason for hiding this comment

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

This is misleading - we should clarify whether "equivalent dtypes" are such that dt1 == dt2, or if simply np.can_cast(dt1, dt2)

Copy link
Member

Choose a reason for hiding this comment

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

Might be tempting to write this in the following form:

>>> np.zeros(3, dtype=dict(names=    ['col1', 'col2'],
...                        formats=  ['i4','f4'],
...                        offsets=  [0, 4],
...                        itemsize= 12)

Copy link
Member

Choose a reason for hiding this comment

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

Which of course, raises the question of whether np.dtype(**dict) should be added as a shorhand for np.dtype(dict)

Copy link
Member

Choose a reason for hiding this comment

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

Aligning the [...] isn't PEP8. It may take a bit of getting used to but eventually the unaligned versions become easier to read.

@charris
Copy link
Member

charris commented Sep 21, 2017

@ahaldane Now that #6053 is in we should get this finished up.

@charris
Copy link
Member

charris commented Sep 24, 2017

@ahaldane ping.

@ahaldane
Copy link
Member Author

Got it, I'll go over it soon.

@ahaldane ahaldane force-pushed the structure_docs branch 2 times, most recently from f07ea87 to 3a7f388 Compare September 25, 2017 21:33
@ahaldane
Copy link
Member Author

Updated, and ready to read through.

You can view an html version of the current state at https://ahaldane.github.io/user/basics.rec.html

@ahaldane ahaldane force-pushed the structure_docs branch 2 times, most recently from 602e22d to 5be7def Compare September 27, 2017 23:33
Copy link
Member

Choose a reason for hiding this comment

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

"array of length two" and omit the comma. The clause after the comma is essential.

Copy link
Member

Choose a reason for hiding this comment

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

"for example, interpreting ..."

Copy link
Member

Choose a reason for hiding this comment

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

comma after "purposes"

Copy link
Member

Choose a reason for hiding this comment

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

Maybe "For simple manipulation of tabular data, other pydata projects, such as pandas, xarray, or DataArray, provide higher-level interfaces that may be more suitable."

Copy link
Member

Choose a reason for hiding this comment

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

"by numpy, but can be manually specified."

Copy link
Member

Choose a reason for hiding this comment

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

Could use a rewrite. I'd probably start a new sentence instead of using "with".

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to number subtitles? Maybe a simple enumerated list would do.

Copy link
Member

Choose a reason for hiding this comment

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

"is an empty string, '', the ..."

Copy link
Member

Choose a reason for hiding this comment

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

"... string and separated ..."

Copy link
Member

Choose a reason for hiding this comment

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

"specifications, all of the same length."

Copy link
Member

Choose a reason for hiding this comment

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

"The optional 'offsets' key is a list of integer byte offsets, one for each field within the structure."

Copy link
Member

Choose a reason for hiding this comment

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

"... to contain all the fields."

Copy link
Member

Choose a reason for hiding this comment

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

Commas rather than parenthesis. I don't know when the current parenthetical scourge originated, but it seems to be everywhere these days :-(

Copy link
Member

Choose a reason for hiding this comment

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

Is there an example of using a scalar array for the rhs? Does scalar array mean 1-D array here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess by "scalar array" I mean "unstructured array", will fix.

Copy link
Member

Choose a reason for hiding this comment

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

Is there an example of that? Which side has the single field, or is that both sides?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll try to modify the example, perhaps using different variable names,.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe "The view's" instead of "This view's".

Copy link
Member

Choose a reason for hiding this comment

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

and <- comma

Copy link
Member

Choose a reason for hiding this comment

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

this <- the

Copy link
Member

Choose a reason for hiding this comment

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

Could omit "Importantly".

Copy link
Member

Choose a reason for hiding this comment

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

"such that" <- "so that".

Copy link
Member

Choose a reason for hiding this comment

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

though <- thought

Copy link
Member

Choose a reason for hiding this comment

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

Omit "of".

Copy link
Member

Choose a reason for hiding this comment

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

What does "equivalent" mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

equal, will fix

Copy link
Member

Choose a reason for hiding this comment

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

dimension <- dimesions.

Copy link
Member

Choose a reason for hiding this comment

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

``True`` ?

Copy link
Member

Choose a reason for hiding this comment

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

What might be the alternative?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently in this case we get:

[1]: a = np.zeros(3, dtype='f,f,f')
[2]: b = np.zeros(3, dtype='f,f')
[3]: a == b
FutureWarning: elementwise == comparison failed and returning scalar instead; this will raise an error or perform elementwise comparison in the future

I'll reword the text to more accurately describe what happens.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe "The .... operators always return ..."?

Copy link
Member

Choose a reason for hiding this comment

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

Many or all? Maybe "no other"?

Copy link
Member

Choose a reason for hiding this comment

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

which <- that.

Copy link
Member

Choose a reason for hiding this comment

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

Comma not needed.

Copy link
Member

Choose a reason for hiding this comment

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

which <- that

@charris
Copy link
Member

charris commented Nov 10, 2017

I'm not sure that we should be using 'title' instead of italicized title or title, the same for other names for parts of the dtype.

NumPy should hire a copy editor, there should be plenty out there in this time of self publishing.

@ahaldane
Copy link
Member Author

Updated, thanks a lot. You make a great copy editor!

For the styling of the parts of the dtype, my idea was to use them as normal nouns through most of the document (eg, "the field name is.."), except in the dtype-specification section where I need to make clear the format of the tuple, in which case I write (fieldname, datatype, shape) and refer to the three variables in the tuple in code-styling like fieldname.

@ahaldane ahaldane force-pushed the structure_docs branch 2 times, most recently from 6ca3fda to 4280fb3 Compare November 10, 2017 02:51
@charris charris merged commit de26584 into numpy:master Nov 11, 2017
@charris
Copy link
Member

charris commented Nov 11, 2017

OK, let's get this in. Thanks Allan.

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.

3 participants