pdoc should display which dostring line is causing the issue if a ValueError is raised on re.split

[dhermangh]

Problem Description

In Returns section, if the numpy docstring is missing a type description, a ValueError is raised. It is fine, however a message displaing which line raised the error should be displayed, helping catch which docstring lines are incorrect. Sometimes linters miss such errors.
For example such docstring:

def foobar(x: str) -> Y:
    """
    I am a docstring in a numpy format

    Parameters
    ----------
    x: str
        I'm a parameter of a str type.

    Returns
    -------
        Tuple containing status message string and CPE, time delta or None
    """

Will raise said ValueError:

Traceback (most recent call last):
  File "/home/qabot/venvs/qabot_venv/bin/pdoc", line 8, in <module>
    sys.exit(cli())
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/__main__.py", line 199, in cli
    pdoc.pdoc(
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/__init__.py", line 529, in pdoc
    out = render.html_module(module, all_modules)
  File "/usr/lib/python3.10/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/render.py", line 106, in html_module
    return env.get_template("module.html.jinja2").render(
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/environment.py", line 1304, in render
    self.environment.handle_exception()
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/environment.py", line 939, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 311, in top-level template code
    {%- if loop.nextitem -%}
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/frame.html.jinja2", line 36, in top-level template code
    {% block body %}
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/frame.html.jinja2", line 42, in block 'body'
    {% block content %}{% endblock %}
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 101, in block 'content'
    {% block module_contents %}
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 104, in block 'module_contents'
    {{ member(m) }}
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/runtime.py", line 782, in _invoke
    rv = self._func(*arguments)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 208, in template
    {{ docstring(doc) }}
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/runtime.py", line 782, in _invoke
    rv = self._func(*arguments)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 212, in template
    <div class="docstring">{{ var.docstring | replace("@public", "") | to_markdown | to_html | linkify(namespace=var.qualname, shorten=False) }}</div>
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/render_helpers.py", line 193, in to_markdown_with_context
    return to_markdown(docstring, module, docformat)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/render_helpers.py", line 198, in to_markdown
    return docstrings.convert(docstring, docformat, module.source_file)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/docstrings.py", line 43, in convert
    docstring = numpy(docstring)
  File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/docstrings.py", line 187, in numpy
    content, tail = re.split(r"\n(?![ \n])", content, maxsplit=1)
ValueError: not enough values to unpack (expected 2, got 1)

The traceback is not helpful, it should contain the exception message itself and the offending docstring line. If it is possible - it should also contain path to the offending file.

Proposal

A message with the offending line should be printed (it would be nice to also have file path to the offending file). As a temp fix in my local envirnoment I've added a try...except block:

try:
  content, tail = re.split(r"\n(?![ \n])", content, maxsplit=1)
except ValueError as e:
  print(content)
  raise

Of course, a better logging/printing should be utilized.

[mhils]

This shouldn't raise in the first place, contributions are welcome! :)

…

On Mon, 18 Nov 2024, 10:24 dhermangh, ***@***.***> wrote: Problem Description In Returns section, if the numpy docstring is missing a type description, a ValueError is raised. It is fine, however a message displaing which line raised the error should be raised, helping catch which docstring lines are incorrect. Sometimes linters miss such errors. For example: ` def foobar(x: str) -> Y: """ I am a docstring in a numpy format Parameters ---------- x: str I'm a parameter of a str type. Returns ------- Tuple containing status message string and CPE, time delta or None """ ` Such a docstring will raise said ValueError: Traceback (most recent call last): File "/home/qabot/venvs/qabot_venv/bin/pdoc", line 8, in <module> sys.exit(cli()) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/__main__.py", line 199, in cli pdoc.pdoc( File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/__init__.py", line 529, in pdoc out = render.html_module(module, all_modules) File "/usr/lib/python3.10/contextlib.py", line 79, in inner return func(*args, **kwds) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/render.py", line 106, in html_module return env.get_template("module.html.jinja2").render( File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/environment.py", line 1304, in render self.environment.handle_exception() File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/environment.py", line 939, in handle_exception raise rewrite_traceback_stack(source=source) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 311, in top-level template code {%- if loop.nextitem -%} File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/frame.html.jinja2", line 36, in top-level template code {% block body %} File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/frame.html.jinja2", line 42, in block 'body' {% block content %}{% endblock %} File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 101, in block 'content' {% block module_contents %} File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 104, in block 'module_contents' {{ member(m) }} File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/runtime.py", line 782, in _invoke rv = self._func(*arguments) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 208, in template {{ docstring(doc) }} File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/jinja2/runtime.py", line 782, in _invoke rv = self._func(*arguments) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/templates/default/module.html.jinja2", line 212, in template <div class="docstring">{{ var.docstring | ***@***.***", "") | to_markdown | to_html | linkify(namespace=var.qualname, shorten=False) }}</div> File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/render_helpers.py", line 193, in to_markdown_with_context return to_markdown(docstring, module, docformat) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/render_helpers.py", line 198, in to_markdown return docstrings.convert(docstring, docformat, module.source_file) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/docstrings.py", line 43, in convert docstring = numpy(docstring) File "/home/qabot/venvs/qabot_venv/lib/python3.10/site-packages/pdoc/docstrings.py", line 187, in numpy content, tail = re.split(r"\n(?![ \n])", content, maxsplit=1) ValueError: not enough values to unpack (expected 2, got 1) The traceback is not helpful, it should contain the exception message itself and the offending docstring line. If it is possible - it should also contain path to the offending file. Proposal A message with the offending line should be printed (it would be nice to also have file path to the offending file). As a temp fix in my local envirnoment I've added a try...except block: try: content, tail = re.split(r"\n(?![ \n])", content, maxsplit=1) except ValueError as e: print(content) raise Of course, a better logging/printing should be utilized. — Reply to this email directly, view it on GitHub <#758>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHY2PSHO6EN55PFQ6ZNKIL2BG56VAVCNFSM6AAAAABR7JL5WSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGY3DQMBQG44DOMQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[dhermangh]

Sure, in that case I'll prepare a PR soon. :)

[tobiscode]

Okay, so now that #784 has been merged, pdoc now throws this error for the example above (substituting tuple for Y):

RuntimeError: Docstring processing failed for docstring=
"""
I am a docstring in a numpy format

Parameters
----------
x: str
    I'm a parameter of a str type.

Returns
-------
    Tuple containing status message string and CPE, time delta or None
"""
source_file=PosixPath('/users/koeh_ti/projects/pdoc/foo.py')
docformat='numpy'

As hinted at by the original comment, the NumPy docstring convention requires there to be a type for both in- and output parameters. Sphinx has an extension that takes advantage of Python type hints but I think it's just for convenience rather than a newer docstring convention, and my understanding is that architecturally, it would be hard for pdoc to do the same.

Following up on #784 (comment):

Generally speaking, if something does not parse as a particular format, we should leave the text as-is.

we would therefore need to determine how to treat the above minimal example (with optionally displaying a warning). This would be just for this project, since it's undefined behavior in general.

Let me start with some opening suggestions:

1. The most specific way to address this issue is to check if an indented block is part of the Returns section, and prepend a dummy type (e.g. a zero-length, non-whitespace character, or a pythonic _):

    if heading != "Returns":
        content, tail = re.split(r"\n(?![ \n])", content, maxsplit=1)
    else:
        content = "\u200b\n" + content
        tail = ""

   would yield

   I am a docstring in a numpy format
   
   ###### Parameters
    - **x** (str):
      I'm a parameter of a str type.
   
   ###### Returns
    - **​**: Tuple containing status message string and CPE, time delta or None

   Pro: It gives the most intuitive fallback for this docstring, i.e., a Returns section with the type missing, but the description still following after the colon.
   Con: It does not handle anything outside the case of Returns section, possibly leaving other failing cases to still throw an error rather than silently failing and falling back to just render the input text.
2. A more generic alternative would be to wrap everything inside for heading, content in zip(sections[1::2], sections[2::2]): into a try-except statement, treating every failure of a section as resulting in passing through the raw NumPy input as markdown syntax. For example:

    try:
        if content.startswith(" "):
            sec_content, tail = re.split(r"\n(?![ \n])", content, maxsplit=1)
        else:
            sec_content, tail = content, ""
        if heading in (...):
            contents += f"###### {heading}\n{_numpy_parameters(sec_content)}"
        elif heading == "See Also":
            contents += f"###### {heading}\n{_numpy_seealso(sec_content)}"
        else:
            contents += f"###### {heading}\n{dedent(sec_content)}"
        contents += tail
    except AnyException as e:
        contents += f"###### {heading}\n{dedent(content)}"

   would yield

   I am a docstring in a numpy format
   
   ###### Parameters
    - **x** (str):
      I'm a parameter of a str type.
   
   ###### Returns
   Tuple containing status message string and CPE, time delta or None

   Pro: Will catch more potential failure cases.
   Con: It might have some unintended side effects, and the result might look inappropriate.
3. Option 3 would be similar to 2, but having the try-except block in the pdoc.docstrings.convert function, such that it applies to all formats.
   Pro: Even more general.
   Con: The docstring of an entire module/function/etc. will look unformatted, even if most parts would be valid.
4. Could still be to just let the error be thrown, since it's invalid NumPy syntax.
   Pro: Strict adherence to the NumPy docstyle.
   Con: Raises the bar that people have to reach in order to have at least a rudimentary documentation.

Personally, I would totally understand Option 4, but I tend to lean towards @mhils 's preference to keep going and parsing whatever's possible, such that pdoc can truly be as simple, forgiving, and beginner-friendly as possible. Since I don't have an overview of how many issues the conversions functions throw (and that get reported), I would personally prefer option 1, but if these codes are prone to break, then a one-fix-fixes-all approach like 2 or 3 would be better.

I probably forgot about some options, so please let me know. At the end, it's the maintainer's choice - I'm happy to implement this functionality whatever gets decided.

Cheers

[mhils]

I've posted a potential fix in #789 up for discussion. :)