Skip to content

Use double quotes for all docstrings, including single-quoted docstrings#9020

Merged
MichaReiser merged 1 commit intomainfrom
enforce-double-quotes-for-all-docstrings
Dec 7, 2023
Merged

Use double quotes for all docstrings, including single-quoted docstrings#9020
MichaReiser merged 1 commit intomainfrom
enforce-double-quotes-for-all-docstrings

Conversation

@MichaReiser
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser commented Dec 6, 2023 •
edited
Loading

Summary

This PR changes our docstring formatting to enforce double quotes for single-quoted docstrings (and not just triple-quoted strings).

I see reasons for and against this.

  • We enforce an empty line after all docstring, regardless if they are single or triple quoted. Meaning, we recognize single-quoted string literals as docstrings (according to PEP 257). This speaks in favor of enforcing double quotes
  • The quote-style documentation mentions that it enforces double quotes for all docstring, but this isn't the case without this PR.
  • PEP 257 recommends using triple double quotes for all docstrings, but this PR only enforces using double quotes, but not using triple quotes. Meaning, the result is only half-correct.

Alternatives:

  • Leave as is
  • Be even more opinionated and change quotes to triple quotes

I favor the consistency of what we consider docstrings and how we format them.

Test Plan

Added test

@MichaReiser
Copy link
Copy Markdown
Member Author

MichaReiser commented Dec 6, 2023 •
edited
Loading

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@MichaReiser MichaReiser added the formatter Related to the formatter label Dec 6, 2023
@MichaReiser MichaReiser force-pushed the enforce-double-quotes-for-all-docstrings branch 2 times, most recently from 0a21edf to bd8b9b0 Compare December 6, 2023 06:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 6, 2023 •
edited
Loading

ruff-ecosystem results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser force-pushed the enforce-double-quotes-for-all-docstrings branch 2 times, most recently from e19ea42 to 9993bb0 Compare December 6, 2023 07:04
@MichaReiser MichaReiser changed the base branch from main to enforce-valid-format-options December 6, 2023 07:04
@MichaReiser MichaReiser mentioned this pull request Dec 6, 2023
Merged
@MichaReiser MichaReiser force-pushed the enforce-double-quotes-for-all-docstrings branch from 9993bb0 to 6286396 Compare December 6, 2023 07:06
@MichaReiser MichaReiser force-pushed the enforce-valid-format-options branch from 42c6891 to fadaddd Compare December 6, 2023 07:09
@MichaReiser MichaReiser changed the title Enforce double quotes for all docstrings Reformat single-quoted docstrings with double quotes Dec 6, 2023
@MichaReiser MichaReiser changed the title Reformat single-quoted docstrings with double quotes Use double quotes for all docstrings, including single-quoted docstrings Dec 6, 2023
@MichaReiser MichaReiser force-pushed the enforce-double-quotes-for-all-docstrings branch from 6286396 to 56cb6ac Compare December 6, 2023 07:10
Base automatically changed from enforce-valid-format-options to main December 6, 2023 07:15
konstin
konstin approved these changes Dec 6, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Be even more opinionated and change quotes to triple quotes

I'd favor this, because single quotes obfuscate that the statement is a docstring

@MichaReiser MichaReiser force-pushed the enforce-double-quotes-for-all-docstrings branch from 56cb6ac to 9cb815b Compare December 7, 2023 04:25
@MichaReiser MichaReiser merged commit 981a070 into main Dec 7, 2023
@MichaReiser MichaReiser deleted the enforce-double-quotes-for-all-docstrings branch December 7, 2023 04:41
@krehel krehel mentioned this pull request Dec 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BurntSushi BurntSushi BurntSushi approved these changes

@charliermarsh charliermarsh charliermarsh approved these changes

@konstin konstin konstin approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MichaReiser @BurntSushi @charliermarsh @konstin