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

Formatter: Docstrings with quote-style = "single" configuration #7615

Closed
alkatar21 opened this issue Sep 23, 2023 · 29 comments · Fixed by #7680
Closed

Formatter: Docstrings with quote-style = "single" configuration #7615

alkatar21 opened this issue Sep 23, 2023 · 29 comments · Fixed by #7680
Assignees
Labels
accepted Ready for implementation formatter Related to the formatter

Comments

@alkatar21
Copy link

Following #1904 (comment) I tried the new configuration

[tool.ruff.format]
# Use single quotes rather than double quotes.
quote-style = "single"

But now docstrings are also formatted with single quotes:

def func():
  '''Docstring.'''
  string_taking_function('A string')

I thought with docstrings there is no real discussion that they always use """ and it only refers to normal strings and PEP 8 also says so. Like the following example:

def func():
  """Docstring."""
  string_taking_function('A string')

There was a short discussion on Discord already where @charliermarsh mentioned the following:

Yeah need to look into this — we’ve discussed it before but started off by using the configured quotes universally.
Worth checking what the single-quote Black forks do here, and if users of our flake8-quotes plug-in tend to use double quotes for docstrings even when enforcing single quotes

For me it would be important that docstrings always use """ regardless of quote-style. I am not quite sure if this is to be understood as a feature request or rather as a bug. D300 considers this as a bug as well.

@konstin konstin added formatter Related to the formatter needs-decision Awaiting a decision from a maintainer labels Sep 23, 2023
@vanchaxy
Copy link

We also use "single everywhere except docstring". +1 to always use double for docstring (preferably) or make it configurable.

@charliermarsh
Copy link
Member

It looks like Blue also uses double quotes for triple-quoted strings and docstrings, and single quotes everywhere else.

@charliermarsh
Copy link
Member

charliermarsh commented Sep 23, 2023

Some data from Code Search:

  • 22 files with docstring-quotes = "single" (source)
  • 57 files with multiline-quotes = "single" (source)
  • 372 files with inline-quotes = "single" (source)

So the majority of projects that are using single quotes only apply it to non-docstring, non-multiline strings, but there are some projects that deviate from that pattern.

@MichaReiser
Copy link
Member

MichaReiser commented Sep 25, 2023

@alkatar21 would you expect that " are also used for non-docstring triple quoted strings?

What about a single-quoted docstring? Should it use " or '':

def test():
    'Single quoted docstring'
   ...

Using " for all triple quoted strings would be in line with PEP8

We can

  • Change quote-style to only apply to not-triple quoted strings
  • and/or add a new quote-style-triple-quoted (or similar)

@alkatar21
Copy link
Author

@alkatar21 would you expect that " are also used for non-docstring triple quoted strings?

I believe the following are all docstrings (used by editors and sphinx for example) and should therefore use ".

class ExampleClass:
    """Class docstring."""
    variable1: int
    """Docstring for variable1."""
    variable2: int
    """Docstring for variable2."""

    def __init__(self, parameter: str) -> None:
        self.parameter: str = parameter
        """Docstring for parameter."""

I don't remember ever using non-docstring triple quoted strings and accordingly have no personal opinion about it, but I would suggest to follow PEP8.

What about a single-quoted docstring? Should it use " or '':

def test():
    'Single quoted docstring'
   ...

I didn't even know that is possible, but off the top of my head I'd say ' since it's a simple string?

Using " for all triple quoted strings would be in line with PEP8

Then I would suggest to follow PEP8.

We can

* Change `quote-style` to only apply to not-triple quoted strings

* and/or add a new `quote-style-triple-quoted` (or similar)

I would suggest to follow PEP8 and implement Change 'quote-style' to only apply to not-triple quoted strings. It is probably easier and well justified with PEP8. If there is a big demand, you can always consider adding the option later. I think it is better to possibly add the option at some point in the future than to regret about the option later on.

@charliermarsh
Copy link
Member

@MichaReiser - the other option is to mirror the configuration options that we use for flake8-quotes: a setting for inline quotes, a setting for multiline quotes, and a setting for docstrings (which applies to single- and triple-quoted docstrings).

@konstin
Copy link
Member

konstin commented Sep 25, 2023

I'd prefer only having at most a single quotes setting

@charliermarsh
Copy link
Member

I agree that it would be nice to have a single setting. The data I scraped above demonstrates that we can’t use a single setting and still support all users, but we could make that choice.

If we want a single setting, my suggestion would be to enforce double quotes for docstrings and triple-quoted strings, and use the string setting for all others. This would be consistent with Blue and Pyink which always use double quotes for such strings.

@MichaReiser
Copy link
Member

@MichaReiser - the other option is to mirror the configuration options that we use for flake8-quotes: a setting for inline quotes, a setting for multiline quotes, and a setting for docstrings (which applies to single- and triple-quoted docstrings).

Yeah, I prefer having as few options as possible. Ideally, just one, two if we must. I would try to avoid having all three.

The other part that's interesting about this is... Should these options now become global? Or are we fine repeating the configuration twice (I would argue it's fine, because the formatter makes the lint rule obsolete)

@tanoc
Copy link

tanoc commented Sep 27, 2023

I think having the same settings as in flake8-quotes is not a bad idea. I'm fine with defaults values for multiline and docstrings checking what is most commonly used, but excluding customization on these settings could create the same problem that fueled the need for Black forks (Blue, Pyink, ...)

@MichaReiser MichaReiser removed the needs-decision Awaiting a decision from a maintainer label Sep 27, 2023
@charliermarsh
Copy link
Member

Our current plan is to always use double quotes for docstrings and triple-quoted strings (as per PEP 8 and PEP 257). We'll change this an upcoming release, prior to the Beta.

If there's still strong demand, we'll then consider adding an additional dedicated setting to use single quotes everywhere (i.e., for docstrings and triple-quoted strings in addition).

@tanoc
Copy link

tanoc commented Sep 27, 2023

Seems reasonable. Just to add to your data, please keep in mind that there a lot of projects using pre-commit with double-quote-string-fixer (that just fixes normal strings and doesn't change docstrings or triple-quoted strings, so maybe is not so black an white as it seems).

@charliermarsh charliermarsh added the accepted Ready for implementation label Sep 27, 2023
@warsaw
Copy link

warsaw commented Sep 27, 2023

I just tried out quote-style = 'single' on one of my repos and hit exactly this problem.

It looks like Blue also uses double quotes for triple-quoted strings and docstrings, and single quotes everywhere else.

Yes, blue explicitly keeps triple-quoted-double-quotes for docstrings.

@charliermarsh
Copy link
Member

👍 We're going to change this, probably in the upcoming release (or the next one, if it doesn't make it).

@warsaw
Copy link

warsaw commented Sep 27, 2023

Great to hear @charliermarsh. I'm super excited about being able to switch to ruff format.

I noticed one other formatting deviation from blue related to right-hanging comments, but I'll open a new ticket on that when I get a chance, if it hasn't already been reported.

@charliermarsh charliermarsh self-assigned this Sep 27, 2023
charliermarsh added a commit that referenced this issue Sep 28, 2023
…7680)

## Summary

At present, `quote-style` is used universally. However, [PEP
8](https://peps.python.org/pep-0008/) and [PEP
257](https://peps.python.org/pep-0257/) suggest that while either single
or double quotes are acceptable in general (as long as they're
consistent), docstrings and triple-quoted strings should always use
double quotes. In our research, the vast majority of Ruff users that
enable the `flake8-quotes` rules only enable them for inline strings
(i.e., non-triple-quoted strings).

Additionally, many Black forks (like Blue and Pyink) use double quotes
for docstrings and triple-quoted strings.

Our decision for now is to always prefer double quotes for triple-quoted
strings (which should include docstrings). Based on feedback, we may
consider adding additional options (e.g., a `"preserve"` mode, to avoid
changing quotes; or a `"multiline-quote-style"` to override this).

Closes #7615.

## Test Plan

`cargo test`
@Destroy666x
Copy link

Destroy666x commented Oct 13, 2023

Please reconsider this as it's a bit nonsensical as of now since you can do:

[tool.ruff.flake8-quotes]
docstring-quotes = 'single'
inline-quotes = 'single'
multiline-quotes = 'single'

and then the formatter is just straight up incompatible with the other part of the software. Software definitely shouldn't conflict with itself.

You could just split it into 3 variables like above for the most compatible approach.

If you need any arguments why use single quotes - they're simply easier to press and that's the advantage. Disadvantages? None for me, I don't mind that let's say 90% of projects go for doubles, no other tool that I'm using has problems with that either.

@Destroy666x
Copy link

Destroy666x commented Oct 20, 2023

Formatter is unrelated to rules, as can be also seen from my comment above. Doing that changed nothing indeed. There's currently no way to disable the quote formatting, pretty sure.

EDIT: comment got deleted in case someone is confused.

@henryiii
Copy link
Contributor

FYI, the disadvantage is that you're much more likely to have single quotes inside a string of text than a double quote. Your comment above has one single quote, and mine has seven. Neither of us used a double quote.

And easier to press doesn't matter if your formatter converts them to double quotes for you anyway. ;)

(That's the reasoning behind Black's choice, by the way)

Though I'd also add that single quotes are used by Python's built in repr's.

@henryiii
Copy link
Contributor

henryiii commented Oct 26, 2023

I was hoping to covert projects like pypa/black and pyproject-metadata to the Ruff formatter, and wasn't able to due to this - previously we used double-quote-string-fixer to convert strings to single quotes, so all our triple quoted strings are single quoted already. I don't really see why someone would want double quoted triple strings if they wanted single quoted strings - but it's a bit hard for me to be sure though, since I'm personally on the double quote "go with black" side. Triple quoted strings actually do not have the one key differentiator for double/single quotes: you can put single or double quotes inside them!

But anyway, I'd like to adopt ruff-format in these projects without having to ask all our developers about changing styles. Though I could ask, given what PEP says.

@henryiii
Copy link
Contributor

henryiii commented Oct 26, 2023

Ahh! double-quote-string-fixer leaves triple quoted strings alone! So we have both types of strings in the codebase, I think that's totally fine to standardize on something, and moving toward more double quotes is good! Not an issue for me anymore. I'll let someone who actually cares about single quotes argue about triple quoted strings if they care. :)

Though I notice all the triple quoted strings that were changed were not docstrings. :(

@Destroy666x
Copy link

Destroy666x commented Oct 26, 2023

FYI, the disadvantage is that you're much more likely to have single quotes inside a string of text than a double quote. Your comment above has one single quote, and mine has seven. Neither of us used a double quote.

That's... quite unreasonable I'd say, as it assumes, I don't know why:

  • usage of shortened forms
  • language - English, which uses ' quite often indeed. Most of other languages don't.
  • language usage in the 1st place, my software could be just a bot with nearly 0 user interaction. Or I could have software with good practice of keeping translations in some config/data-like files, e.g. YAML or JSON. This would mean most of strings are programming-related and unlikely to have any of ' or ", unless parsing of specific formats occurs, but then e.g. dealing with JSON would mean mostly dealing with ".

You just can't generalize like that, so still no idea why software vendors/authors try to enforce such things TBH, especially when introducing inconsistencies like I mentioned. And the formatters utilize double minimal escaping quoting method anyways, so wrapping with the other when necessary occurs. It really shouldn't matter what's used and what's the user's preference.

And easier to press doesn't matter if your formatter converts them to double quotes for you anyway. ;)

True. I could just prefer the more minimalistic look of ' though, including while reading the code in repository, why not. I'm not that pedantic but some people might be.

@henryiii
Copy link
Contributor

henryiii commented Oct 26, 2023

I'm not sure I'd say "most other" - At least Spanish and French both use single quotes, and pretty much don't use double quotes at all! («» used instead). Languages that don't use either don't matter, since either is fine. Also, Python is written in English. I think it's enough to break the symmetry; it doesn't need to be a huge reason since otherwise it's a toss-up, it's just a character choice. And if you are dealing with formatted intput/output like json you should use a library (factor out the quoting) and not try to manually handle quoting. :)

Also, many other (computer) languages don't give you the option, and those mostly force double quotes (C, C++, Java, Rust, Matlab, Go, Swift) for strings. Using double quotes in Python is also consistent with those languages, and makes it easier to go back and forth. (note: JavaScript and Fortran are like Python. Ruby and Bash allow both, but double quotes interpolate and single do not. I don't know of a language that forces single quotes.)

Anyway, doesn't really matter here, I think here the thing to argue is if you like single quotes, do you like triple quoted single quotes? Due to PEP 257, enforcing double quoted docstrings seems fine, but what about free-standing single triple quotes? That's what we have (hopefully had) in pypa/build. Docstrings used double, everything else (even if triple quoted) was single. I don't like single quotes anyway, so I'm a bad judge. :)

@warsaw
Copy link

warsaw commented Oct 26, 2023

including while reading the code in repository

Yes, exactly. It's not just about the ease of writing single quotes over double quotes (at least for US qwerty keyboards, don't know about others), but about reading code. The extra screen grit of double quotes can reduce the readability of code too.

My understanding was that black adopted this style because it was more compatible with JavaScript or JSON or some such, with little ergonomic consideration.

@Destroy666x
Copy link

Destroy666x commented Oct 26, 2023

I'm not sure I'd say "most other" - At least Spanish and French both use single quotes, and pretty much don't use double quotes at all

I don't think any of Asian languges does and that's already the most as there are loads of variations. While French uses them as frequently as English, I don't know where you see them in Spanish - I personally almost never do. They're mostly popular in Germanic languages I'd say.

Also, Python is written in English. I think it's enough to break the symmetry

Why does that matter at all...? Not only does Python allow both, but also the language used by authors is quite irrelevant IMO, while we're talking about a specific tool especially.

And if you are dealing with formatted intput/output like json you should use a library (factor out the quoting) and not try to manually handle quoting

This point makes no sense, as I was obviously talking from perspective of someone e.g. writing the librar to parse the JSON.

Using double quotes in Python is also consistent with those languages

Why would I want to be consistent with entirely different languages though? Python is normally way way more loose than C, C++ or Java and rather uncomparable. The fact that C doesn't have lambdas doesn't mean I should avoid lambda usage, for instance. The fact that languages differ is an advantage of variety of them, not a disadvantage. You can code a quick bot without type hints and without caring much about formatting with Python and then a business application with 1000 classes with Java that follows very specific standards.

do you like triple quoted single quotes?

I don't mind them at all ¯_(ツ)_/¯ E.g. for the reason specified by the person above.

I think you're way too focused on defending a specific style while not considering the freedom aspect at all.

@henryiii
Copy link
Contributor

henryiii commented Oct 27, 2023

I think this discussion shows why Black didn't make it configurable, and just picked a "correct" one. :) Happy to argue about why double quotes are better somewhere else, but it doesn't have anything to do with this issue - Ruff-format already lets you pick single quotes!

To summarize: if you set quote-style = "single", you get this:

def f():
    """
    Docstring.
    """

    single = 'hello'
    triple = """Hi there"""

The docstring is correct, PEP 257 states docstrings should use double quotes. The triple quote string is correct according to PEP 8, which states all triple quoted strings should be made with double quotes. I'm okay with Ruff not providing configurability to contradict a PEP - I'll try to get it updated in pypa/build and pyproject-metadata. The one place where it's left open by formatting PEPs is for simple strings, and Ruff has an option for that, which is nice. Though IMO, even just seeing them above, I'd much rather just make the simple string double quoted too. ;)

Having a '"preserve"` option would be fantastic (didn't notice it was missing!), and would help people do whatever they are doing now with Black. Being at least as configurable as Black is a low bar. ;)

@MichaReiser
Copy link
Member

Having a '"preserve"` option would be fantastic (didn't notice it was missing!), and would help people do whatever they are doing now with Black. Being at least as configurable as Black is a low bar. ;)

preserve would be slightly different to Black's --skip-string-normalization in the sense that it still removes unnecessary escapes and normalizes string prefixes. It's the same in that it keeps your quotes the same. However, I want to wait with adding it until we figure out how to handle the new string preview feature that automatically joins and splits strings. The formatter could get the quotes wrong when splitting or joining strings depending on how obscure the quote rules are in a project.

@ThiefMaster
Copy link
Contributor

I'd also like to be able to configure quote styles separately. flake8-quotes does it very nicely, so why not allow autoformatting to be as flexible:

inline-quotes = single
multiline-quotes = single
docstring-quotes = double
avoid-escape = true

@ThiefMaster
Copy link
Contributor

Actually, if you want a single setting: How about a quotes = match-linter option? That way you have onle ONE setting and it uses the config of the flake8-quotes rules...

@MichaReiser
Copy link
Member

We're considering adding the preserve option in #8822. Please comment on the PR if you use --skip-string-normalization and your use case isn't covered by Ruff's settings today nor after adding the preserve option. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation formatter Related to the formatter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants