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

Regression in 20.8b1: "skip-string-normalisation" is ignored for docstrings #1639

Closed
exhuma opened this issue Aug 27, 2020 · 4 comments
Closed
Labels
R: duplicate This issue or pull request already exists T: bug Something isn't working

Comments

@exhuma
Copy link

exhuma commented Aug 27, 2020

Describe the bug

As of version 20.8b1, the "skip string normalisation" setting is ignored, but only for docstrings

To Reproduce Steps to reproduce the behavior:

Example code

class Hello:
    '''
    Wrong Strings
    '''


def foo():
    '''
    wrong-string
    '''
  1. Run Black on it with these arguments '....'
black --check --diff -S foo.py
  1. See error
--- foo.py      2020-08-27 06:15:44.100921 +0000
+++ foo.py      2020-08-27 06:16:07.598050 +0000
@@ -1,10 +1,10 @@
 class Hello:
-    '''
+    """
     Wrong Strings
-    '''
+    """


 def foo():
-    '''
+    """
     wrong-string
-    '''
+    """
would reformat foo.py
Oh no! 💥 💔 💥
1 file would be reformatted.

Expected behavior

While it is true that single-quoted strings violate PEP-8 via PEP-257 I would still prefer if that the behaviour of black would not change in that respect. In 19.10b0 it was still honouring the "skip-string-normalisation" setting, even for docstrings. This has now changed.

Environment (please complete the following information):

  • Version: 20.8b1
  • OS and Python version: Linux/Python 3.7.0

Does this bug also happen on master?

yes

Additional context

Our team has a fairly large number of Python projects. I just checked my own listing and I have over 170 projects myself, so I assume our team will have easily over 200 projects. Most small libraries, but some are fairly large code-bases. A lot of code conventions are historic and we're switching to black at the moment. The skip-string-normalisation was/is a big helping hand in that migration, as it would otherwise just simply cause too many diffs & thus conflicts.

This option allowed us to have a fairly painless migration to black as we were quite close to PEP-8 already. But the string-normalisation hamstringed us a bit. And now with 20.8b1 it's back. And this morning, coming to the office we were showered with CI-emails of our daily runs with pipeline failures 😦

@exhuma exhuma added the T: bug Something isn't working label Aug 27, 2020
@TylerYep
Copy link

I believe this has already been fixed in this commit

@exhuma
Copy link
Author

exhuma commented Aug 27, 2020

This is good to know. Thanks for the info.

In case anyone else comes across this, I have fixed our pipelines for now by blacklisting both 20.8b0 and 20.8b1 using the following command:

pip install --pre "black != 20.8b0, != 20.8b1"

Instead of pinning this using either < 20.8b0 or ==19.10b0, the blacklisting syntax will ensure we get any upcoming release of black in our daily pipelines.

Considering that this seems to be handled in the commit mentioned above, I think it's safe enough to close this issue with the workaround I just laid out.

If it breaks again in a newer release I will notice fairly quickly and update this ticket if need be.

@hugovk
Copy link
Contributor

hugovk commented Aug 27, 2020

To avoid dozens of CI failure emails, I'd suggest pinning formatters and linters like Black and Flake8 to a single, specific version on CI and locally. And then making explicit upgrades from time to time.

@ambv
Copy link
Collaborator

ambv commented Aug 27, 2020

Closing as duplicate of #1634.

@ambv ambv closed this as completed Aug 27, 2020
@ambv ambv added the R: duplicate This issue or pull request already exists label Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R: duplicate This issue or pull request already exists T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants