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

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

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Dec 6, 2023

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
Member Author

MichaReiser commented Dec 6, 2023

@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
Copy link
Contributor

github-actions bot commented Dec 6, 2023

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 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
Copy link
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
17 checks passed
@MichaReiser MichaReiser deleted the enforce-double-quotes-for-all-docstrings branch December 7, 2023 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants