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

gh-100101: Clarify documentation of zip()'s strict option #100103

Merged
merged 3 commits into from
Dec 28, 2022

Conversation

JustAnotherArchivist
Copy link
Contributor

@JustAnotherArchivist JustAnotherArchivist commented Dec 8, 2022

Issue: #100101

  • Straightforward docs clarification change, no news entry
  • Needs application to the 3.10 and 3.11 docs.

@netlify
Copy link

netlify bot commented Dec 8, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit d60b638
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/63918b34d3e53b0009a7191d
😎 Deploy Preview https://deploy-preview-100103--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Dec 8, 2022
@JustAnotherArchivist
Copy link
Contributor Author

Looks like doctest does not support having both output and an exception in an example, but that's the entire point here: showing that the loop body is executed up to the point where the error condition (one iterable being exhausted) is hit. Any ideas?

@slateny
Copy link
Contributor

slateny commented Dec 8, 2022

You could try a try/except and print the exception instead (assuming it does work).

@JustAnotherArchivist
Copy link
Contributor Author

Yeah, I suppose that would work around it, but it'd also add significant noise to the example (five lines instead of two). :-/

@JustAnotherArchivist
Copy link
Contributor Author

For reference, this is the old open issue about doctest not supporting this: #65382. (Thanks, han-solo!) Looks like I have finally found a real-world stdlib use case for this feature. :-)

@slateny
Copy link
Contributor

slateny commented Dec 8, 2022

In that case, maybe just a comment above the line saying what it'll print (or the length) would work to be less noisy, though at that point the output wouldn't exactly match the display so also not too certain about that.

@JustAnotherArchivist
Copy link
Contributor Author

The only alternative I can think of is disabling the test for that example using # doctest: +SKIP, accompanied by a comment about that issue (not certain, but I think rST supports that).

@JustAnotherArchivist
Copy link
Contributor Author

There is in fact precedent for skipping such doctests (although I'm not entirely sure why it was necessary there), so I'm going to propose doing the same thing here and enabling the test again in the future when doctest gains that functionality. In my opinion, a clear example of the exact zip(..., strict=True) behaviour is more important than a passing doctest in this case.

@brandtbucher brandtbucher self-requested a review December 27, 2022 23:15
@brandtbucher brandtbucher self-assigned this Dec 27, 2022
@brandtbucher brandtbucher added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Dec 28, 2022
@brandtbucher brandtbucher merged commit cf1c098 into python:main Dec 28, 2022
@miss-islington
Copy link
Contributor

Thanks @JustAnotherArchivist for the PR, and @brandtbucher for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@brandtbucher
Copy link
Member

Thanks!

@bedevere-bot
Copy link

GH-100594 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Dec 28, 2022
@bedevere-bot
Copy link

GH-100595 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 28, 2022
…GH-100103)

(cherry picked from commit cf1c098)

Co-authored-by: JustAnotherArchivist <JustAnotherArchivist@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Dec 28, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 28, 2022
…GH-100103)

(cherry picked from commit cf1c098)

Co-authored-by: JustAnotherArchivist <JustAnotherArchivist@users.noreply.github.com>
miss-islington added a commit that referenced this pull request Dec 28, 2022
(cherry picked from commit cf1c098)

Co-authored-by: JustAnotherArchivist <JustAnotherArchivist@users.noreply.github.com>
miss-islington added a commit that referenced this pull request Dec 28, 2022
(cherry picked from commit cf1c098)

Co-authored-by: JustAnotherArchivist <JustAnotherArchivist@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants