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

ruff format removes trailing whitespace in doctests #10275

Open
Tracked by #13371
peterjc opened this issue Mar 7, 2024 · 5 comments
Open
Tracked by #13371

ruff format removes trailing whitespace in doctests #10275

peterjc opened this issue Mar 7, 2024 · 5 comments
Labels
bug Something isn't working formatter Related to the formatter

Comments

@peterjc
Copy link

peterjc commented Mar 7, 2024

Testing under Linux, using test_case.py based on a real world example:

test_case.py.txt

"""Silly example using whitespace in doctests.

This defines a function returning a string with trailing space:

>>> pad("X", 3)
'   X   '

This can be used directly in doctests like so:

>>> print(pad("X", 3))
   X   

You can't see it, but there should be three trailing spaces
after the X, and if an editor removes them it breaks the test.
"""


def pad(text, spaces=1):
    """Add the given string with spaces before and after.

    Example:

    >>> print(pad("Barry"))
     Barry 
    >>> pad("John", 2)
    '  John  '

    That's it.
    """
    return " " * spaces + text + " " * spaces


if __name__ == "__main__":
    import doctest

    print("Running doctests...")
    doctest.testmod(verbose=True)
    print("Done")

Using the current version of black, the module docstring is unchanged (good), but the function doctest is broken (bad):

$ black --version && black --diff test_case.py
black, 24.2.0 (compiled: yes)
Python (CPython) 3.10.12
--- test_case.py        2024-03-07 14:54:01.979428+00:00
+++ test_case.py        2024-03-07 14:57:08.562391+00:00
@@ -19,11 +19,11 @@
     """Add the given string with spaces before and after.

     Example:

     >>> print(pad("Barry"))
-     Barry 
+     Barry
     >>> pad("John", 2)
     '  John  '

     That's it.
     """
would reformat test_case.py

All done! ✨ 🍰 ✨
1 file would be reformatted.

This seems to be a long standing bug in black psf/black#1654 although the difference in module level versus function level docstrings might be new.

However with ruff, both the module docstring and the function docstring are broken (two doctests fail):

$ ruff --version && ruff format --diff test_case.py
ruff 0.3.1
--- test_case.py
+++ test_case.py
@@ -8,7 +8,7 @@
 This can be used directly in doctests like so:

 >>> print(pad("X", 3))
-   X   
+   X

 You can't see it, but there should be three trailing spaces
 after the X, and if an editor removes them it breaks the test.
@@ -21,7 +21,7 @@
     Example:

     >>> print(pad("Barry"))
-     Barry 
+     Barry
     >>> pad("John", 2)
     '  John  '


1 file would be reformatted

I agree that it is better to avoid meaningful trailing white space, but it can result in some ugly workarounds (assuming the code cannot be changed due to backward compatibility).

Indeed the GitHub editor remove the trailing spaces too when pasting in the code! I have restored them by hand.

I would like both ruff format and black not to touch trailing whitespace in the output section of a doctest.

Separately flake8 or ruff lint can warn about W291, and the real examples this is based on use # noqa: W291 on the docstring.

@charliermarsh charliermarsh added the formatter Related to the formatter label Mar 7, 2024
@MichaReiser MichaReiser added the bug Something isn't working label Mar 8, 2024
@MichaReiser
Copy link
Member

I would like both ruff format and black not to touch trailing whitespace in the output section of a doctest.

That makes sense to me and we do have the infrastructure to support when using docstring-code-format. @BurntSushi do you know how where / how complicated it would be to preserve trailing whitespace in doctest assertion lines?

@BurntSushi
Copy link
Member

I took a couple minutes to look, but it's not obvious to me where the issue is. One thing that I'd wonder about is, if we're reformatting docstring code snippets and reformatting itself will strip whitespace, then I'm not sure we can preserve trailing whitespace while docstring-code-format is enabled.

Otherwise, when docstring-code-format is disabled, I'm not sure I see any explicit stripping of trailing whitespace from a line in a docstring before printing it. So I'd wonder if this is being done at a lower level of the formatter? But I'm not sure.

@MichaReiser
Copy link
Member

MichaReiser commented Mar 8, 2024

Otherwise, when docstring-code-format is disabled, I'm not sure I see any explicit stripping of trailing whitespace from a line in a docstring before printing it. So I'd wonder if this is being done at a lower level of the formatter? But I'm not sure.

Oh yeah, that could be it, depending on how we print the trailing space. The Printer doesn't print any whitespace. It defers it until you print the next (non whitespace) character. Although that only applies for space() not when writing a string.

Edit: Nope, not it. You can see how the trailing whitespace is missing in the written IR after the X. So it must be happening somewhere 😆 https://play.ruff.rs/ded4925d-c8be-408e-951d-c51e18c52a9b

@MichaReiser
Copy link
Member

MichaReiser commented Mar 8, 2024

@BurntSushi I believe it's coming from this trim_end call

let trim_end = line.line.trim_end();
if trim_end.is_empty() {
return if line.is_last {
// If the doc string ends with ` """`, the last line is
// ` `, but we don't want to insert an empty line (but close
// the docstring).
Ok(())
} else {
empty_line().fmt(self.f)
};
}

We would probably need a way for the line to inform whether the trailing whitespace should be trimmed or not and we would set this to false for the prompt output. But I'm a bit at a loss of where that would have to happen.

@MichaReiser
Copy link
Member

I investigated this a little more. We would need to extend our doctest formatting also to handle the expected output and print that as verbatim instead of using print_one. I don't think doing this would be very complicated, but is probably a day or two effort.

Links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working formatter Related to the formatter
Projects
None yet
Development

No branches or pull requests

4 participants