- Notifications
You must be signed in to change notification settings - Fork 54
Adv indexing #45
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
Adv indexing #45
Conversation
Looks good overall. Here's what I've got: L66: This is probably not worth anything, but your callout box says "Definition: Advanced Indexing" and then doesn't actually define anything. It just describes cases in which your indexing is advanced. L91: "See that we can acces its an arbitrary number of the arrays contents" -- remove the word its L105: "The specification for accessing" sounds a little weird to me. I think of a specification more as a spec sheet. Maybe the instruction or something? L141: you run L231: "we specified a single index-array array" -- get rid of one of the L233: "The corresponding entries of each of the N index arrays is used" should be are used L246: "We specify three index-arrays, the indices to be..." replace that comma with a colon: "three index-arrays: the indices..." L252: "The other index arrays are formed accordingly." -- I would say "formed similarly" instead L383: "...by supplying N index-arrays of integers, one for each axis of data." -- replace the comma with a semicolon L392: Up to you, but Boolean Array Indexing can be read as Boolean-Array Indexing (indexing into boolean arrays) or Boolean Array-Indexing (using booleans for array indexing). I'd hyphenate where you want the reader to read a hyphen in (Boolean Array-Indexing). L393: "...permits the use of a boolean-value array" should be "boolean-valued" L418: "...and L437: "Use boolean array indexing NumPy's logical functions" is a mess. Here's a rewrite (also changes
L450: "Converting a Boolean Index Array to Integer Index Arrays" should be Boolean Index-Array (hyphenated) L451: "...can be used to take a boolean-value array" should be "boolean-valued" L468: "...a tuple of three integer-value index..." should be "integer-valued" L471: not necessary, but split the line where that comma is and just get rid of the comma: # unpacking the arrays is not necessary # you can use the tuple as an index >>> ind0, ind1, ind2 = np.where(bool_ind) >>> x[ind0, ind1, ind2] At this same point, it might be worth just showing that you don't have to unpack by adding this line: >>> x[np.where(bool_ind)] L478: "where specific conditions were met" should be "are met" L494: "boolean array indexing" should be "boolean array-indexing" L562: "Recall that redundant entries in array" -- "redundant entries in an array" L603: Not important, but I would say Combining Basic and Advanced Indexing instread of Advanced and Basic L604: "Integer and boolean-valued arrays" should be "Integer- and boolean-valued arrays" -- note the hyphen on integer L607: again, not important but I would swap the order on advanced and basic Run a consistency check for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ Up there
@davidmascharka thanks so much for this! 0.11 will be released in a few minutes |
@davidmascharka can you proof this?