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

Update code sample when importing modules in queue doc #94244

Merged
merged 1 commit into from
Jul 1, 2022

Conversation

Mariatta
Copy link
Member

@Mariatta Mariatta commented Jun 24, 2022

In the queue documentation, the code snippet shows the import to be not PEP 8 compliant.

https://docs.python.org/3/library/queue.html

Since people typically copy-paste from such code samples, I think it's important to show best-practices here.

In the queue documentation, the code snippet shows the import to be not PEP 8 compliant.

Since people typically copy-paste from such code samples, I think it's important to show best-practices here.
@Mariatta Mariatta requested a review from rhettinger as a code owner June 24, 2022 20:36
@bedevere-bot bedevere-bot added awaiting core review docs Documentation in the Doc dir labels Jun 24, 2022
@Mariatta Mariatta added skip issue skip news docs Documentation in the Doc dir needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes and removed docs Documentation in the Doc dir labels Jun 24, 2022
@rhettinger
Copy link
Contributor

We've generally avoided this sort of change. It doesn't improve the readability of the docs because we want people to focus on the main part of the code sample.

Also the rationale behind the PEP 8 guidance doesn't apply here. In production code, having imports on separate lines makes maintenance easier because diffs are simpler to read. That of course doesn't apply to doc examples. The other rationale is that if there are many imports, then having them on separate lines and in alphabetical order makes them easier to search.

@ambv
Copy link
Contributor

ambv commented Jul 1, 2022

I think you're both right. In general, we avoid overshadowing the code example with boilerplate as much as possible. Agreed. In this case however, it takes barely a 6% SLOC difference to split the import into two and in this way to avoid the busy import statement becoming its own (bad) "code example". So I'd say it's worth the tiny churn it takes to land the three PRs.

@ambv ambv merged commit ad55147 into main Jul 1, 2022
@ambv ambv deleted the split-queue-imports branch July 1, 2022 15:51
@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 1, 2022
In the queue documentation, the code snippet shows the import to be not PEP 8 compliant.

Since people typically copy-paste from such code samples, I think it's important to show best-practices here.
(cherry picked from commit ad55147)

Co-authored-by: Mariatta Wijaya <Mariatta@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 1, 2022
@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 1, 2022
In the queue documentation, the code snippet shows the import to be not PEP 8 compliant.

Since people typically copy-paste from such code samples, I think it's important to show best-practices here.
(cherry picked from commit ad55147)

Co-authored-by: Mariatta Wijaya <Mariatta@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 1, 2022
@bedevere-bot
Copy link

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

ambv pushed a commit that referenced this pull request Jul 1, 2022
…-94490)

In the queue documentation, the code snippet shows the import to be not PEP 8 compliant.

Since people typically copy-paste from such code samples, I think it's important to show best-practices here.
(cherry picked from commit ad55147)

Co-authored-by: Mariatta Wijaya <Mariatta@users.noreply.github.com>
ambv pushed a commit that referenced this pull request Jul 1, 2022
…-94491)

In the queue documentation, the code snippet shows the import to be not PEP 8 compliant.

Since people typically copy-paste from such code samples, I think it's important to show best-practices here.
(cherry picked from commit ad55147)

Co-authored-by: Mariatta Wijaya <Mariatta@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 issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants