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

Issue in case of excess Ellipsis ... at the end of a multi-line doctest statement #148

Open
etienneschalk opened this issue Feb 5, 2024 · 6 comments

Comments

@etienneschalk
Copy link

If there is a trailing ellipsis at the end of a doctest statement

My IDE syntax coloring, and various other examples in the xarray codebase suggests me that there should not be any trailing Ellipsis at the end of a multi-line doctest statement, but I thought it was still worth mentioning this edge case!

Screenshot: Diff before and after running pytest-accept

Screenshot from 2024-02-05 23-58-03

@max-sixty
Copy link
Owner

Thanks for the issue @etienneschalk .

Could you clarify the issue? What is the difference between the current output and the output you'd want?

@etienneschalk
Copy link
Author

Hello @max-sixty

Regarding the current output and output I would want, there is a bug as the last line, here, *empty* is duplicated and not replaced properly. I fixed the issue by removing the trailing ....

By trailing ... I mean the last line of a multi-line doctest statement being empty, only prefixed with ... before the output.

Concrete example: on the xarray repo: here is a docstring with a trailing ...
https://github.com/pydata/xarray/blob/f33a632bf87ec29dd9346f9b01ad4eec2194f72a/xarray/core/options.py#L255

    >>> with xr.set_options(display_width=40):
    ...     print(ds)
    ...  <-- the trailing `...`

Let's say I break the docstring...

    >>> with xr.set_options(display_width=40):
    ...     print(ds)
    ... 
-   <xarray.Dataset>
+   <xarray.Dataset> I break the doctest!
    Dimensions:  (x: 1000)
    Coordinates:
      * x        (x) int64 8kB 0 1 ... 999
    Data variables:
        *empty*

...and then run pytest-accept on the file:

pytest --doctest-modules xarray/core/options.py --accept
    >>> with xr.set_options(display_width=40):
    ...     print(ds)
-   ...
    <xarray.Dataset>
    Dimensions:  (x: 1000)
    Coordinates:
      * x        (x) int64 8kB 0 1 ... 999
    Data variables:
        *empty*
+       *empty*

We can see that the trailing ... was eaten, and the last line *empty* was duplicated. It looks like an off-by-one error

In the doctest official documentation we can see several examples of trailing ...: for instance https://docs.python.org/3/library/doctest.html#how-are-docstring-examples-recognized
So I would say that pytest-accept should ideally accept these trailing ..., or if it auto-removes them, at least not duplicate the last line.

It looks similar to the known issues involving the backslashes in docstrings eg #146

@max-sixty
Copy link
Owner

Ah, thanks a lot for explaining. That does look weird.

Generally pytest-accept will ignore any ... and just replace the whole docstring. So it's unclear why it fails here.

Would you have a small self-contained example you could paste in? Then we have a shared example that I can attempt to fix.

@etienneschalk
Copy link
Author

The following example should be copy/pastable into a single file. The doctest has a single trailing Ellipsis, and the output is wrong on purpose. Output is True when we expect it to be False. The scenario is that pytest-accept would update the docstring accordingly, replacing True by False.

File before running pytest-accept:

def negate(value: bool) -> bool:
    """This function negates its input

    >>> negate(True)
    ...
    True
    """

    return not value

File after pytest-accept ran on it:

def negate(value: bool) -> bool:
    """This function negates its input

    >>> negate(True)
    False
    True
    """

    return not value

@max-sixty
Copy link
Owner

max-sixty commented Feb 7, 2024

Thanks for the excellent example. I spent some time looking into this.

Unfortunately, I think it's a problem with python's stdlib.

Specifically in this example...

def negate(value: bool) -> bool:
    """This function negates its input

    >>> negate(True)
    ...
    True
    """

...failure.example.want is 'True\n', rather than '...\nTrue\n'.

So in pytest-accept, we start writing from the start of the example (i.e. we overwrite ...), and continue for as many lines as the new example provides. But the final line of the existing example isn't overwritten, because our output is a line short. And I don't think there's a way of us figuring that out, since we can't explicitly identify the end of the example.


The only way I could see identifying this is to attempt to parse the whole docstring (in this case failure.test.docstring='This function negates its input\n\n >>> negate(True)\n ...\n True\n '), but that would be really error prone.


Let's leave this open for a while. The next step is to put a bug report into python itself.

@etienneschalk
Copy link
Author

etienneschalk commented Feb 17, 2024

Hello,

In the context of pydata/xarray#8760 I try to use the ruff formatter to format docstrings.

This is an opt-in behaviour introduced in https://astral.sh/blog/ruff-v0.1.8 enabled by adding in pyproject.toml:

[tool.ruff.format]
docstring-code-format = true

I ran pre-commit run --all on the xarray repo and noticed that all the trailing Ellipsis of the project were removed. For instance in git diff:

         >>> def adder(data, arg):
         ...     return data + arg
-        ...
         >>> def div(data, arg):
         ...     return data / arg
-        ...
         >>> def sub_mult(data, sub_arg, mult_arg):
         ...     return (data * mult_arg) - sub_arg
-        ...

So it means, fixing this issue might be unneeded if formatters that remove excess Ellipsis were to be generalized in various projects!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants