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: Minor tweak to advanced indexing example in tutorial #1550

Merged
merged 5 commits into from
Nov 24, 2023

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Oct 26, 2023

I just wanted to make a very minor suggestion I had while reading through the tutorial (which is very nice, thank you for that!):

In the advanced indexing section, the array used in the 1D examples is np.arange(10). This means that the indices and corresponding values are the same. There's nothing wrong with this of course, but I thought the example might be more immediately grokkable if a different example array were used where the values were different than the indices. I arbitrarily chose np.arange(10) ** 2, but any other choice would be equally valid. This is of course a very minor point and pretty subjective, so please feel free to ignore!

I also added two other minor commits that popped up while running through the contributor guide to make the PR:

  • I noticed virtualenv was referenced, but the example uses the built-in venv module, so I removed it, and
  • Fixed a failing doctest due to a malformed comment.

Again - nothing major; please feel free to ignore/cherry-pick as you see fit!

Suggestion to modify the advanced indexing example so
that the indices and the values in the array differ.
@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Oct 26, 2023
@d-v-b
Copy link
Contributor

d-v-b commented Oct 26, 2023

Looks good to me! I'm going to merge in the next few days unless anyone else objects.

@MSanKeys963
Copy link
Member

MSanKeys963 commented Oct 27, 2023

Hi @rossbar. 👋🏻
Good to see you here! 😄

Thanks for sending the PR. Everything looks good to me.
PS. If you have any additional suggestions (anywhere in docs/tutorials), please let us know.

@joshmoore
Copy link
Member

👍 from my side as well. Only question I have, @rossbar, is whether or not you'd like to add yourself to the release notes. The more the merrier!

@MSanKeys963
Copy link
Member

Added release notes for this in #1574.
Good to go with this.

@joshmoore joshmoore merged commit b93860a into zarr-developers:main Nov 24, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs release notes Automatically applied to PRs which haven't added release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants