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: Created an indexing how-to #20093

Merged
merged 4 commits into from
Nov 17, 2021
Merged

DOC: Created an indexing how-to #20093

merged 4 commits into from
Nov 17, 2021

Conversation

Mukulikaa
Copy link
Contributor

Addresses most points in #19586.

Looking forward to any feedback regarding best practices or other use-cases that should be included!

cc: @melissawm @rossbar

Copy link
Member

@melissawm melissawm left a comment

Choose a reason for hiding this comment

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

Thanks for getting started on this @Mukulikaa ! I feel like we could have a couple more examples, and maybe we should link to Nicolas Rougier's exercises?

doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
Comment on lines 75 to 84
>>> arr = np.arange(3*4).reshape(3, 4)
>>> column_indices = [[1, 3], [0, 2], [2, 2]]
>>> np.arange(arr.shape[0])[:, np.newaxis]
array([[0],
[1],
[2]])
>>> arr[np.arange(arr.shape[0])[:, np.newaxis], column_indices]
array([[ 1, 3],
[ 4, 6],
[10, 10]])
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 a good example, but I feel like it is a bit cryptic. Maybe expand a little bit on the motivation or print a few of the steps to make it easier for the reader?

Use :ref:`slicing-and-striding` to access chunks of an array.
But if you want to access multiple scattered elements to create
complicated subsets, you have to use :ref:`advanced-indexing`. Use
:func:`ix_` to quickly contruct index arrays::
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 at some point we had discussed not encouraging the use of ix_ - @rossbar do you remember this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember specifically, but I generally agree.

Comment on lines 147 to 148
Replace values
--------------
Copy link
Member

Choose a reason for hiding this comment

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

Is it on purpose that this is a sub-header to "Filter values"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes... My idea was "after you filter the values you can even replace them this way".

doc/source/user/how-to-index.rst Show resolved Hide resolved
@Mukulikaa
Copy link
Contributor Author

The rendered page is here.

Copy link
Member

@melissawm melissawm left a comment

Choose a reason for hiding this comment

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

Hi @Mukulikaa, I have a few notes but nothing major - I think this is good and any further improvements can be done in a follow up, so I'm approving. Thanks again for doing great work!

doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
Index columns
-------------

Use :ref:`dimensional-indexing-tools` to avoid shape mismatches::
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is very clear on first read - maybe show what would happen if you used a "naive" approach here first?

doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
doc/source/user/how-to-index.rst Outdated Show resolved Hide resolved
@Mukulikaa
Copy link
Contributor Author

Thanks for your help, @melissawm!

==========================================

Use :ref:`basic-indexing` features like :ref:`slicing-and-striding`, and
:ref:`dimensional-indexing-tools`.
Copy link
Member

@mattip mattip Nov 17, 2021

Choose a reason for hiding this comment

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

The rendered page for the latest commit https://23276-908607-gh.circle-artifacts.com/0/doc/build/html/user/how-to-index.html is showing italic text for all the output. Does the line before the indent need a : or is a code formatting directive needed to force <code> format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is always like that? I checked a couple of other examples. I didn't use : in some places because I didn't think it was appropriate...

Copy link
Member

Choose a reason for hiding this comment

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

ok. let's put this in as-is, we can always iterate on the formatting.

@mattip mattip merged commit b95cfa5 into numpy:main Nov 17, 2021
@mattip
Copy link
Member

mattip commented Nov 17, 2021

Thanks @Mukulikaa

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

4 participants