ProjectPythia / pythia-foundations

Jupyterbook source for the Foundations collection
http://foundations.projectpythia.org
Apache License 2.0
59 stars 42 forks source link

Clarify Array Slicing Syntax from 'array[start:stop[:step]]' to 'array[start:stop:step]' #465

Closed kazakhpunk closed 1 month ago

kazakhpunk commented 2 months ago

This pull request updates the array slicing syntax documentation. The original syntax notation 'array[start:stop[:step]]' could be misinterpreted as suggesting the step parameter is nested within the stop parameter. This update simplifies the notation to 'array[start:stop:step]', clearly delineating each parameter, enhancing readability and reducing potential confusion for users.

review-notebook-app[bot] commented 2 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

github-actions[bot] commented 2 months ago

👋 Thanks for opening this PR! The Cookbook will be automatically built with GitHub Actions. To see the status of your deployment, click below. 🔍 Git commit SHA: 66ece31a54324654a0b241c4cdac895deb47c0b5 ✅ Deployment Preview URL: https://ProjectPythia.github.io/pythia-foundations/_preview/465

dopplershift commented 2 months ago

The challenge with this update is now it implies that the second colon is mandatory, which isn't true. That's why :step was in its own brackets before.

In fairness, neither the numpy docs nor the Python docs denote slicing as we have previously done here, so maybe this is a good cleanup to match those sources.

kazakhpunk commented 2 months ago

Thank you for the feedback. I appreciate the concern about the notation implying that the second colon and the step parameter are mandatory.

I propose we adopt 'array[start:stop:step]' as the primary notation in examples but include a clear note in the text explaining that the step parameter is optional: "The step parameter is optional and can be omitted, in which case the slice uses a default step of 1."

kazakhpunk commented 1 month ago

Thank you for this feedback!

I've changed array[start:stop:step]' toarray[start:stop:step]`

brian-rose commented 1 month ago

Merging this now. Thanks for the contribution @kazakhpunk!