Add argument to tell docformatter to be compatible with black
weibullguy opened this issue · 15 comments
As pointed out in #39, black inserts a space before a one-line or the summary in a multi-line docstring when it begins with a quote (") character. Default behavior for docformatter is to remove this space, which leads to a never-ending loop. The newly created argument --pre-summary-space will retain this black-inserted space at the expense of adding a space before every docstring, which black will then remove resulting in, yet again, a never-ending loop.
Add an argument (e.g., --black) to make docformatter only add the space when the line begins with a quote character when the --pre-summary-space argument is set. Thus, the invocation for black compatibility would be:
docformatter --pre-summary-space --black
Other than PEP 257, we occasionally move quotes. I'm not entirely sure when Black does move quotes, but it's pretty rare and it's probably more an edge case than codified behaviour. For more see psf/black#1632 and psf/black#3044.
For posterity, here what the documentation has to say on docstrings:
Black also processes docstrings. Firstly the indentation of docstrings is corrected for both quotations and the text within, although relative indentation in the text is preserved. Superfluous trailing whitespace on each line and unnecessary new lines at the end of the docstring are removed. All leading tabs are converted to spaces, but tabs inside text are preserved. Whitespace leading and trailing one-line docstrings is removed.
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#strings
cc @JelleZijlstra am I forgetting another way we format docstrings?
Should this be closed by #46?
Maybe we can start by adding an entry for docformatter in Black integration documentation: https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html
Now I would love to have a dedicated --black option to handle all the details. In the mean time, my own set of options for compatibility is:
$ docformatter --recursive --in-place --wrap-summaries 88 --wrap-descriptions 88 .
As for the never-ending loop, I addressed it by running docformatter before black in a single GitHub Action job: https://github.com/kdeldycke/workflows/blob/f7dd058dd0281f31d04c2192b277f1d6fb727677/.github/workflows/autofix.yaml#L120-L132
@thejcannon I don't think so. #46 adds a space before the summary regardless of what the first character in the summary is. Black only adds the space if the first character is a double quote ". The plan for the --black option is to tell docformatter you only want a pre-summary space when the first character is a double quote (maybe if it's a single or double quote, I'm open to feedback).
Currently --pre-summary-space adds the space regardless of the first character. The --pre-summary-space option requested in #39 was for the existing behavior. The --black flag (not the '80's band) represents a new use case.
@kdeldycke here is a snippet from my docformatter "road map" notes:
Passing --black should set the following:
--pre-summary-space (only if docstring begins with ")
--wrap-descriptions 88
--wrap-summaries 88
Or the pyproject.toml equivalent:
[tool.docformatter]
pre-summary-space = true
wrap-descriptions = 88
wrap-summaries = 88
Thanks @weibullguy for comparing notes about the details of black-compatible options. You just confirmed my initial findings! :)
Having just run into the incompatibility between black and docformatter 1.6, I'm really happy to see the work being done here. The suggestions that @weibullguy added are great, but I would also like to point out that black has configurable line length and other options. Should a black compatible docformatter inherit those black config options from the pyproject.toml, or should the user need to manually match the config options?
Put an attempt to address this issue above, hoping to get feedback on if its on the right track before finishing it
Should a black compatible docformatter inherit those black config options from the pyproject.toml, or should the user need to manually match the config options?
Other than line length, what other black options would docformatter need to accommodate?
Put an attempt to address this issue above, hoping to get feedback on if its on the right track before finishing it
I've provided some feedback in the PR.
@weibullguy Line length is the only thing I know of that is problematic.
For the latest version (v1.6.0-rc5), docformatter removes whitelines after block comments.
For example, the following is how black formats the code:
class MyClass:
def do_stuff(self):
"""Does stuff."""
def do_more(self):
"""Does more."""
def next_method():
return 1
After applying Docformatter:
class MyClass:
def do_stuff(self):
"""Does stuff."""
def do_more(self):
"""Does more."""
def next_method():
return 1
DocFormatter version v1.5.1 seems to be the latest version that keeps the empty lines, keeping it compatible with black.
@gerbenoostra that is correct. Per PEP-257, There’s no blank line either before or after the docstring. Put a pass or any other statement in each method and the blank line won't be stripped.
I see I have another tool interfering. I also use autoflake (mainly to remove unused imports). And that removes the pass lines
But the root problem is Black not adhering to all PEP's, at least not PEP-257.
@NickHilton take a look at the black behavior leaving the blank lines in empty functions/methods. Try to include it in #166. I could also be convinced to add a new argument (e.g., --retain-blank-lines) for docformatter to retain blank lines in empty functions/methods and then have the --black argument set this true. See also #161.
Will take a look - Just pushed the other changes for #166 which I'm hoping make all the unittests pass for the behaviour other than this blank line thing.
Will hopefully get to look at that by Wednesday