kaxil commented on code in PR #76701:
URL: https://github.com/apache/airflow/pull/76701#discussion_r4514400559
##########
providers/common/ai/src/airflow/providers/common/ai/toolsets/hook.py:
##########
@@ -502,20 +502,30 @@ def _extract_description(method: Callable[..., Any]) ->
str:
# Matches Sphinx-style `:param name:` and Google-style `name:` under an
``Args:`` block.
-_SPHINX_PARAM_RE = re.compile(r":param\s+(\w+):\s*(.+?)(?=\n\s*:|$)",
re.DOTALL)
+_SPHINX_PARAM_RE = re.compile(r"^(\s*):param\s+(\w+):(.*)$")
_GOOGLE_ARGS_RE = re.compile(r"^\s{2,}(\w+)\s*(?:\(.+?\))?:\s*(.+)",
re.MULTILINE)
def _parse_param_docs(docstring: str) -> dict[str, str]:
"""Parse parameter descriptions from Sphinx or Google-style docstrings."""
params: dict[str, str] = {}
- # Try Sphinx style first.
- for match in _SPHINX_PARAM_RE.finditer(docstring):
- name = match.group(1)
- desc = " ".join(match.group(2).split())
- params[name] = desc
+ # Try Sphinx style first. As in a reStructuredText field list, a
description runs on
+ # through the lines indented deeper than its ``:param`` line, so neither a
continuation
+ # line starting with a role such as ``:class:`` nor an example block after
the last
+ # parameter changes where it ends.
+ name: str | None = None
+ indent = 0
+ for line in docstring.splitlines():
+ if match := _SPHINX_PARAM_RE.match(line):
+ name, indent = match.group(2), len(match.group(1))
+ params[name] = match.group(3)
+ elif name and (not line.strip() or len(line) - len(line.lstrip()) >
indent):
Review Comment:
This cuts `GoogleDisplayVideo360Hook.erf_uri`'s `entity_type` down to "The
type of file Partner, Advertiser, InsertionOrder,", because its second line
sits at the same indent as the `:param` line. The old regex kept the whole
list. The docstring is what's wrong there, so could you indent that line in
`display_video.py` in this PR? `partner_id` just above it is also missing its
colon, so neither parser picks it up.
##########
providers/common/ai/tests/unit/common/ai/toolsets/test_hook.py:
##########
@@ -782,6 +782,58 @@ def test_sphinx_style(self):
assert result["name"] == "The name of the thing."
assert result["count"] == "How many items."
+ def test_sphinx_last_param_stops_at_dedented_text(self):
Review Comment:
None of the new cases has a description that continues after a blank line,
so deleting the `not line.strip()` check still passes every
`TestParseParamDocs` test while cutting `S3Hook.copy_object`'s
`source_bucket_key` to "The key of the source object." Could you add a case
shaped like that one (`:param`, a blank line, an indented second paragraph,
then another `:param`)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
