Validate docstring examples

[mathias-baumann-frequenz]

• Update stand-alone scripts to be import-friendly
• Add module to validate ``` python wrapped code examples in docstrings
• Fix docstring code example problems
• Patch @Actor decorator to retain the origins class' doc, name and module metadata

fixes #81

[leandro-lucarella-frequenz]

I have a few comments, but amazing if now the examples are being tested and all really fixed!

If you really did a research and there are no tools available doing this, I'm thinking we might just create a repo for the tool, it looks like something that could be reused by other people and that the ecosystem is missing. But I'm also fine to add it here and split it in the future.

Also, please remove the "fixes #..." because we are still missing the testing of >>> blocks, right? Or split the issue in 2.

[leandro-lucarella-frequenz]

I guess this doesn't catch class attributes or class attributes? Does it catch properties? It also doesn't catch module-level variables.

Any reason not to basically look for __doc__ in any symbol (as long as the symbol is originated in this module)?

[mathias-baumann-frequenz]

From what I learned only a few symbols have the doc property, namely modules, functions, methods and classes.

Well, that's not exactly correct, vars have the property, but it contains type documentation rather than user documentation.

E.g.

>>> x = 3
>>> dir(x)
['__abs__', '__add__', '__and__', '__bool__', '__ceil__', '__class__', '__delattr__', '__dir__', '__divmod__', '__doc__', '__eq__', '__float__', '__floor__', '__floordiv__', '__format__', '__ge__', '__getattribute__', '__getnewargs__', '__gt__', '__hash__', '__index__', '__init__', '__init_subclass__', '__int__', '__invert__', '__le__', '__lshift__', '__lt__', '__mod__', '__mul__', '__ne__', '__neg__', '__new__', '__or__', '__pos__', '__pow__', '__radd__', '__rand__', '__rdivmod__', '__reduce__', '__reduce_ex__', '__repr__', '__rfloordiv__', '__rlshift__', '__rmod__', '__rmul__', '__ror__', '__round__', '__rpow__', '__rrshift__', '__rshift__', '__rsub__', '__rtruediv__', '__rxor__', '__setattr__', '__sizeof__', '__str__', '__sub__', '__subclasshook__', '__truediv__', '__trunc__', '__xor__', 'as_integer_ratio', 'bit_count', 'bit_length', 'conjugate', 'denominator', 'from_bytes', 'imag', 'numerator', 'real', 'to_bytes']
>>> x.__doc__
"int([x]) -> integer\nint(x, base=10) -> integer\n\nConvert a number or string to an integer, or return 0 if no arguments\nare given.  If x is a number, return x.__int__().  For floating point\nnumbers, this truncates towards zero.\n\nIf x is not a number or if base is given, then x must be a string,\nbytes, or bytearray instance representing an integer literal in the\ngiven base.  The literal can be preceded by '+' or '-' and be surrounded\nby whitespace.  The base defaults to 10.  Valid bases are 0 and 2-36.\nBase 0 means to interpret the base from the string as an integer literal.\n>>> int('0b100', base=0)\n4"

[leandro-lucarella-frequenz]

Ah, interesting. Well, the docs extracting tool (mkdocstrings) is considering strings following a variable a docstring, I guess it does it by parsing the Python file then. See for example:

• https://frequenz-floss.github.io/frequenz-sdk-python/v0.20/reference/frequenz/sdk/timeseries/#frequenz.sdk.timeseries.UNIX_EPOCH

• frequenz-sdk-python/src/frequenz/sdk/timeseries/_base_types.py

  Lines 13 to 14 in d3528e9

  	UNIX_EPOCH = datetime.fromtimestamp(0.0, tz=timezone.utc)
  	"""The UNIX epoch (in UTC)."""

I guess for the first version it is OK to ignore them, but we should support it in the future.

This also makes me think that testing the documentation should probably be part of building the documentation (or an option), so maybe in the future we could plug it to mkdocstring (making it a plugin or something). This will also ensure that the documentation we get to extract examples is the same as the one that will be rendered. The guy doing mkdocstrings seems very nice, he even replied to some issue in our repo when discussing about it.

[leandro-lucarella-frequenz]

How is it with sybil? I guess it also not considering strings after a variable declaration a docstring for the variable, right?

[leandro-lucarella-frequenz]

@Marenz ping, not sure if you could look into this. Is not super high prio, because I think it is much less likely to have examples here, but still it would be good to know. If it's not supported we can make an extra effort to try to avoid examples in these cases.

[Marenz]

I believe it is supported, as Sybil doesn't use the __doc__ property but parses the files itself

[leandro-lucarella-frequenz]

Can you confirm this? I think it will be important to decide if we allow writing examples in variable documentation or not.

[leandro-lucarella-frequenz]

So, this is amazing. I love the idea of making examples run in the same import context as the file they live in!

What I don't like at all is that this is not really a pytest plugin, it is just hijacking the configuration file!

There is zero reason to run this in the context of pytest, right? You are not using any pytest facilities or features, right?

If so, just create a separate file. You don't even have to make it executable for now, we can just call it using python -m lint-code-examples or whatever.

I think this should be definitely a separate tool, so IMHO the next step would be to move it into its own repo.

[Marenz]

There is zero reason to run this in the context of pytest, right?

I liked that it re-uses the existing interface and error reporting

[leandro-lucarella-frequenz]

I liked that it re-uses the existing interface and error reporting

Oh, really? Can you show an example output? I thought the output was just whatever pylint outputs. In any case IMHO it definitely can't be in conftest.py, maybe if it is a real pytest plugin. But still is hard for me to see an advantage over just producing one file per example and then run pylint (and other linting tools) there, that will also use the normal output of the tools. We can even reuse the option and all the noxfile paraphernalia if we just write examples in a new directory like examples/.generated/ or something like that.

[Marenz]

❰marenz❙~/frequenz/frequenz-sdk-python(git:validate_docstring_examples)❱✘≻ pytest src
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.2, pytest-7.2.2, pluggy-1.0.0
rootdir: /home/marenz/frequenz/frequenz-sdk-python, configfile: pyproject.toml
plugins: anyio-3.6.2, mock-3.10.0, asyncio-0.21.0, time-machine-2.9.0
asyncio: mode=Mode.AUTO
collected 17 items

src/frequenz/sdk/actor/_decorator.py ..                                                                                                                [ 11%]
src/frequenz/sdk/actor/power_distributing/power_distributing.py .                                                                                      [ 17%]
src/frequenz/sdk/power/_distribution_algorithm.py ..........                                                                                           [ 76%]
src/frequenz/sdk/timeseries/_moving_window.py ..                                                                                                       [ 88%]
src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py .                                                                                       [ 94%]
src/frequenz/sdk/timeseries/logical_meter/_logical_meter.py .                                                                                          [100%]

[Marenz]

And here an example with a failure:

❰marenz❙~/frequenz/frequenz-sdk-python(git:validate_docstring_examples)❱✘≻ pytest src
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.2, pytest-7.2.2, pluggy-1.0.0
rootdir: /home/marenz/frequenz/frequenz-sdk-python, configfile: pyproject.toml
plugins: anyio-3.6.2, mock-3.10.0, asyncio-0.21.0, time-machine-2.9.0
asyncio: mode=Mode.AUTO
collected 17 items

src/frequenz/sdk/actor/_decorator.py .F                                                                                                                [ 11%]
src/frequenz/sdk/actor/power_distributing/power_distributing.py .                                                                                      [ 17%]
src/frequenz/sdk/power/_distribution_algorithm.py ..........                                                                                           [ 76%]
src/frequenz/sdk/timeseries/_moving_window.py ..                                                                                                       [ 88%]
src/frequenz/sdk/timeseries/_formula_engine/_formula_engine.py .                                                                                       [ 94%]
src/frequenz/sdk/timeseries/logical_meter/_logical_meter.py .                                                                                          [100%]

========================================================================== FAILURES ==========================================================================
______________________________________________________________ _decorator.py line=127 column=1 _______________________________________________________________

Example at /home/marenz/frequenz/frequenz-sdk-python/src/frequenz/sdk/actor/_decorator.py, line 127, column 1 did not evaluate as expected:
Pylint validation failed for code example:
import asyncio
import inspect
import logging
from typing import Any, Generic, Optional, Type, TypeVar
from frequenz.sdk._internal._asyncio import cancel_and_await
from frequenz.sdk.actor._decorator import *

from frequenz.channels import Receiver, Sender, Broadcast
from frequenz.channels.util import Select
@actor
class Actor1:
    def __init__(
        self,
        name: str,
        recv: Receiver[bool],
        output: Sender[bool],
    ) -> None:
        self.name = name
        self._recv = recv
        self._output = output
    async def run(self) -> None:
        async for msg in self._recv:
            await self._output.send(msg)
@actor
class Actor2:
    def __init__(
        self,
        name: str,
        recv: Receiver[bool],
        output: Sender[bool],
    ) -> None:
        self.name = name
        self._recv = recv
        self._output = output
    async def run(self) -> None:
        async for msg in self._recv:
            await self._output.send(msg)
async def main() -> None:
    input_chan: Broadcast[bool] = Broadcast("Input to A1")
    a1_chan: Broadcast[bool] = Broadcast("A1 stream")
    a2_chan: Broadcast[bool] = Broadcast("A2 stream")
    a_1 = Actor1(
        name="ActorOne",
        recv=input_chan.new_receiver(),
        output=a1_chan.new_sender(),
    )
    a_2 = Actor2(
        name="ActorTwo",
        recv=a1_chan.new_receiver(),
        output=a2_chan.new_sender(),
    )
    a2_rx = a2_chan.new_receiver()
    await input_chan.new_seder().send(True)
    msg = await a2_rx.receive()
asyncio.run(main())

Error: Command '['pylint', '--disable', 'missing-module-docstring,missing-class-docstring,missing-function-docstring,wildcard-import,reimported,unused-import,unused-variable,unused-wildcard-import,no-name-in-module,await-outside-async', '--from-stdin', '/home/marenz/frequenz/frequenz-sdk-python/src/frequenz/sdk/actor/_decorator.py']' returned non-zero exit status 2.
Output: ************* Module sdk.actor._decorator
src/frequenz/sdk/actor/_decorator.py:174:10: E1101: Instance of 'Broadcast' has no 'new_seder' member; maybe 'new_sender'? (no-member)

-------------------------------------------------------------------
Your code has been rated at 8.53/10 (previous run: 10.00/10, -1.47)


_decorator.py:127: SybilFailure
================================================================== short test summary info ===================================================================
FAILED src/frequenz/sdk/actor/_decorator.py::line:127,column:1
=============================================================== 1 failed, 16 passed in 21.83s ================================================================

[Marenz]

I was kind of avoiding to generate actual files and tried to keep it all in memory, most for performance reasons.

[Marenz]

Recreated under my new username at #384