Skip to content

Add slicing tag#2285

Merged
ChrisLovering merged 4 commits into
python-discord:mainfrom
C-Ezra-M:Keyacom-patch-1
Nov 2, 2022
Merged

Add slicing tag#2285
ChrisLovering merged 4 commits into
python-discord:mainfrom
C-Ezra-M:Keyacom-patch-1

Conversation

@C-Ezra-M
Copy link
Copy Markdown
Contributor

@C-Ezra-M C-Ezra-M commented Oct 4, 2022

This file adds a tag with an explanation on sequence slicing.

Added file to another branch because it could interfere with the `main` branch when a pull request is made.
This file adds an explanation on sequence slicing.
@HassanAbouelela
Copy link
Copy Markdown
Contributor

Hello, thanks for writing this up! Is there an issue for this tag?

@C-Ezra-M
Copy link
Copy Markdown
Contributor Author

C-Ezra-M commented Oct 4, 2022

I wrote this because after I helped a person in a help channel, I thought that it would be useful to write a slice syntax tag, even if there is no such issue about it in python-discord/meta.

The conversation:
IMG_20221005_003301.jpg

The message where I mentioned the slice syntax (link):
https://discord.com/channels/267624335836053506/776184661902491678/1026550769358487573

Also, I used the Discohook editor to visualize the embed, and deleted all the stuff I didn't need.

@HassanAbouelela
Copy link
Copy Markdown
Contributor

Alright, for now you can keep this PR open, but please open an issue on the meta repository detailing what it is you think should be added and why. Linking to this PR is a good idea to explain the what.

@HassanAbouelela HassanAbouelela added s: stalled Something is blocking further progress p: 2 - normal Normal Priority a: tags Related to bot tags labels Oct 4, 2022
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 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 writing this, the structure of this tag is good, although I think some of the information isn't necessary. It's best to keep tags short and snappy, otherwise people wont read them.

I've tried condensing the tag down a bit, could you take a look and see what you think and maybe merge both our versions togther?

---
aliases: ["slice", "seqslice", "seqslicing", "sequence-slice", "sequence-slicing"]
embed:
    title: "Sequence slicing"
---
*slicing* is a way of accessing a part of a sequence by specifying a start index, stop index, and step to take elements at.

The syntax for slicing is: `sequence[start:stop:step]`. Each argument can be left out to use a default value, which is `1` for `step`, and is each end of the sequence (index `0` and `len(sequence)`), for `start` and `stop`.

Like sequence indexing, negative integers for the `start` and `stop` will take the nth index *from the end* of the sequence. A negative `step` value will reverse the sequence.

**Examples**
```py
>>> l = [1, 2, 3, 4]
>>> l[2:]
[3, 4]
>>> l[:2]
[1, 2]
>>> l[::-1]
[4, 3, 2, 1]
>>> l[:]
[1, 2, 3, 4]
>>> l[::2]
[1, 3]
```
```py
>>> s = 'abcDeFgHi'
>>> s[-2:2:-2]
'HFD'
>>> s[100:]
''
```

Comment thread bot/resources/tags/slicing.md Outdated
Comment thread bot/resources/tags/slicing.md Outdated
Comment thread bot/resources/tags/slicing.md Outdated
@wookie184 wookie184 added s: waiting for author Waiting for author to address a review or respond to a comment and removed s: stalled Something is blocking further progress labels Oct 21, 2022
@C-Ezra-M
Copy link
Copy Markdown
Contributor Author

Thanks for writing this, the structure of this tag is good, although I think some of the information isn't necessary. It's best to keep tags short and snappy, otherwise people wont read them.

I've tried condensing the tag down a bit, could you take a look and see what you think and maybe merge both our versions togther?

Yes, I consider that feasible, but keep in mind that sentences must start with a capital letter. In line 6:

- *slicing* is a way of accessing a part of a sequence by specifying a start index, stop index, and step to take elements at.
+ *Slicing* is a way of accessing a part of a sequence by specifying a start index, stop index, and step to take elements at.

Other than that, I'm blown away at your proposal and I'm definitely accepting it.

@Xithrius Xithrius added s: needs review Author is waiting for someone to review and approve and removed s: waiting for author Waiting for author to address a review or respond to a comment labels Oct 26, 2022
@wookie184
Copy link
Copy Markdown
Contributor

Yes, I consider that feasible, but keep in mind that sentences must start with a capital letter. In line 6:

- *slicing* is a way of accessing a part of a sequence by specifying a start index, stop index, and step to take elements at.
+ *Slicing* is a way of accessing a part of a sequence by specifying a start index, stop index, and step to take elements at.

Other than that, I'm blown away at your proposal and I'm definitely accepting it.

Thanks :) And yeah that capital letter should be fixed.

Would you be able to commit the changes to this PR so others can review it and we can hopefully get it merged?

@swfarnsworth
Copy link
Copy Markdown
Contributor

Thank you for taking the initiative to create this tag. I think slicing is a great candidate for having a tag, and that we've been without one for too long.

We need to be very intentional about what and how much information is conveyed through a tag. In that light, there's a lot that I'd like to see different about your proposed tag. Let me propose this alternative:

---
aliases: ["slice", "seqslice", "seqslicing", "sequence-slice", "sequence-slicing"]
embed:
    title: "Sequence slicing"
---
*slicing* is a way of accessing a part of a sequence by specifying a start, stop, and step. As with normal indexing, negative numbers can be used to count backwards.

**Examples**
```py
>>> letters = ['a', 'b', 'c', 'd', 'e', 'f', 'g']
['a', 'b', 'c', 'd', 'e', 'f', 'g']
>>> letters[2:]  # from element 2 to the end
['c', 'd', 'e', 'f', 'g']
>>> letters[:4]  # up to element 4
['a', 'b', 'c', 'd']
>>> letters[3:5]  # elements 3 and 4 -- the right bound is not included
['d', 'e']
>>> letters[2:-1:2]  # Every other element between 2 and the last
['c', 'e']
>>> letters[::-1]  # The whole list in reverse
['g', 'f', 'e', 'd', 'c', 'b', 'a']
>>> words = "Hello world!"
>>> words[2:7]  # Strings are also sequences
"llo w"
```

I changed the elements from integers to strings so that the distinction between indices and elements would be especially clear to beginners.

While it's interesting that slices can return empty sequences rather than raising an IndexError, I don't think beginners are likely to discover this or find it confusing if they do. I also think that's too much detail for a tag.

I don't think knowing about the behavior of [:] or [::] is useful, since .copy() is always preferred.

@C-Ezra-M
Copy link
Copy Markdown
Contributor Author

@swfarnsworth Your approach could probably be the best one yet.

Did things according to what was proposed by @swfarnsworth, and the following:
- Fixed the REPL example (assignments never return)
- The word "slicing" at the start was not capitalized in the proposal, even though it started a sentence. I fixed that.
@C-Ezra-M
Copy link
Copy Markdown
Contributor Author

Note about the potentially confusing commit summary (because I can't edit it):

- Fixed the REPL example (assignments never return)

Assignment expressions (a := b) do return.

Copy link
Copy Markdown
Contributor

@swfarnsworth swfarnsworth left a comment

Choose a reason for hiding this comment

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

I have no other requests other than removing the two lines indicated. Thank you for integrating my suggestions, and again for taking the initiative to create this tag!

Comment thread bot/resources/tags/slicing.md Outdated
@ChrisLovering ChrisLovering merged commit 1c1810b into python-discord:main Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tags Related to bot tags p: 2 - normal Normal Priority s: needs review Author is waiting for someone to review and approve

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants