-
Notifications
You must be signed in to change notification settings - Fork 2k
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
autosummary table gets confused by long / complex type hints #6311
Comments
Redacted previous comment, sorry for noise. I need to spend more time looking at the function grammar, I made assumptions about ordering that I apparently cannot. I think |
Sorry for late response. I just posted #6344 to fix this.
Unfortunately, sig is not a valid python code. It sometimes takes an inspected code like |
Ah interesting. I should be able to test the PR tonight but it looks good to me from inspection! |
In this case, we could cheat the system if we can rely on |
Sharing the test code that I was playing with back then for As said previously, I think the easy solution is to find out how to fix #6344. I can revisit this in the future when I have more time. I just figured I'd make this code available publicly in case anybody wants to step in and see if they can get it to work with inspected objects. We'd need to pre-process #!/usr/bin/env python3
import ast
import itertools
def raw_signature(sig):
# create a valid function signature to parse with ast module.
function = "def foo{sig}: pass\n".format(sig=sig)
parsed = ast.parse(function)
# parsed will have a body that is the module, with one element: the function
parsed_function = parsed.body[0]
# Gather all function parameter names without any annotations. Store the
# col_offset as the first index so that argument order can be preserved by
# sorting.
all_args = [] # type: List[Tuple[int, str]]
# args.args: list of positional arguments
if parsed_function.args.args:
for item in parsed_function.args.args:
all_args.append((item.col_offset, item.arg))
# args.vararg: variadic pack (e.g., *args), * is removed from argument name
# so we add it back.
if parsed_function.args.vararg:
all_args.append((
parsed_function.args.vararg.col_offset,
"*{0}".format(parsed_function.args.vararg.arg)
))
# args.kwonlyargs: keyword only args (e.g., after a '*,')
if parsed_function.args.kwonlyargs:
for item in parsed_function.args.kwonlyargs:
all_args.append((item.col_offset, item.arg))
# args.kwarg: variadic keyword pack (e.g., **kwargs), ** is removed from
# argument name so we add it back.
if parsed_function.args.kwarg:
all_args.append((
parsed_function.args.kwarg.col_offset,
"**{0}".format(parsed_function.args.kwarg.arg)
))
# Sort based off the col_offset, but keep the name.
ordered_arguments = [item[1] for item in sorted(all_args)]
COMPARE = True
if COMPARE:
unordered = [item[1] for item in all_args]
print("unordered: ", ", ".join(unordered))
# TODO: this becomes a limited_join
return ", ".join(ordered_arguments)
if __name__ == "__main__":
print("==> no parameters:")
print(raw_signature("()"))
print("==> no annotations:")
print(raw_signature("(boom, blam, blah)"))
print("==> (stage: str, *args, **kwargs)")
print(raw_signature("(stage: str, *args, **kwargs)"))
# TODO: what are the rules????
print(raw_signature("(stage: str, x=12, *args, x=11, **kwargs)"))
print("==> *, thingy")
print(raw_signature("(x=12, *, y=13, **kwargs)"))
print(raw_signature("(foo: str, *, alpha: int = 2, beta: str = 'hi')"))
sig = "(path: Union[pathlib.Path, str], pattern: str, repl: Union[Callable[[Match[AnyStr]], str], str], count: int = 0, flags: int = 0, backup_extension: str = '.orig', line_based: bool = False, demand_different: bool = True, encoding: Optional[str] = None) -> pathlib.Path"
print("==> complicated example:")
print(raw_signature(sig)) |
I found other case. sphinx/tests/test_ext_autosummary.py Lines 43 to 44 in 165897a
I don't know where such signatures come from. But it has been supported by IMO, current approach is not good. autosummary already have raw python function (or method) object once. So we can generate better signature from it. We don't need to modify signature string. Note: Users can override the signature of functions (or methods) by docstring when |
@pv do you happen to know what these test cases for autosummary are? TL;DR this thread is trying to find a way to remove If we removed them, would that be OK or do they serve a specific purpose? There's a couple other tests like this that don't seem to be "valid python function signatures", we're a little confused about what they represent. |
You can do it by overriding signature via docstring (when
I don't know why this is useful. But it is current behavior. |
@svenevs: that's the output produced by numpy ufunc and f2py docstring generators. The notation means positional-only arguments --- at the time I think Python had not converged on a common convention on these. I'm not immediately sure if this has been changed in numpy/f2py by now. Supplying the signature on the first line of the docstring is necessary for extension modules (eg written in C), since Python usually cannot inspect the signatures of the functions provided by them. So, it would be useful to not break this if there's no pressing need to. |
…complex type hints
…complex type hints
…complex type hints
…mplex_typehints2 Fix #6311: autosummary: autosummary table gets confused by complex type hints
Fixed by #6361. |
Yay! Thank you very much for the help and information, I learned about some interesting new features 🙂 |
Describe the bug
While creating the table for
.. autosummary::
, the functionsphinx/sphinx/ext/autosummary/__init__.py
Line 430 in 15bc5a3
will not produce the expected results, some type hints remain, albeit broken up. Here is the long function signature in question:
In the
.. autosummary::
table, the following is produced:The rendered docs can be found online here, but the root of the problem can be seen by the
path
andrepl
parameters (first and third parameter).Stepping through, what actually happens is that everything just gets split on
', '
:sphinx/sphinx/ext/autosummary/__init__.py
Lines 448 to 454 in 15bc5a3
So when we have something like
Union[Path, str]
or the (rather ugly) more complicatedUnion[Callable[[Match], str], str]
, splitting on', '
will give us something like this (splitting each argument on line to make clear):This is actually a pretty hard problem to solve with regular expressions, if it's even possible to do so (unless I'm missing something, it's kind of late xD). I think we would have to start counting how many
[
we have seen and match them with]
(after having seen a:
).So I was playing around with alternatives, found a nice SO answer that has some relevant stuff, here's a rough prototype:
Right now I believe what
args
andopts
in the current autosummary code are doing is trying to shoveopts
at the end if there is space (since there's a maximum length here). I'm not sure why this happens, since all of the keyword arguments have to be last anyway right?Basically we can use
ast
and just limited join on the transformed thing if we don't care about necessarily preserving existing behavior. But I'm not sure about the implications of all this ... so thought I would open up the issue for discussion of other possible solutions or maybe downsides to useast
(it definitely doesn't come for free, but it's also a simple parse).To Reproduce
Steps to reproduce the behavior:
Expected behavior
No stray
str]
end up as a "parameter" in the autosummary table.Your project
https://github.com/svenevs/ci_exec
Screenshots
Environment info
The text was updated successfully, but these errors were encountered: