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

Improvements to stubs generated from docstrings #6368

Merged
merged 4 commits into from Feb 24, 2019
Merged
Changes from 3 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -22,9 +22,14 @@

class ArgSig:
"""Signature info for a single argument."""

_TYPE_RE = re.compile(r'^[a-zA-Z_][\w\[\], ]*(\.[a-zA-Z_][\w\[\], ]*)*$')

def __init__(self, name: str, type: Optional[str] = None, default: bool = False):
self.name = name
self.type = type
if type and not self._TYPE_RE.match(type):
raise ValueError("Invalid type: " + type)
self.type = type if type and self._TYPE_RE.match(type) else None
This conversation was marked as resolved by wiktorn

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 19, 2019

Member

This smells redundant. Why match twice?

# Does this argument have a default value?
self.default = default

@@ -129,8 +134,13 @@ def add_token(self, token: tokenize.TokenInfo) -> None:

if token.string == ')':
self.state.pop()
self.args.append(ArgSig(name=self.arg_name, type=self.arg_type,
default=bool(self.arg_default)))
try:
self.args.append(ArgSig(name=self.arg_name, type=self.arg_type,
default=bool(self.arg_default)))
except ValueError:
# wrong type, use Any
self.args.append(ArgSig(name=self.arg_name, type=None,
default=bool(self.arg_default)))
self.arg_name = ""
self.arg_type = None
self.arg_default = None
@@ -193,7 +203,15 @@ def infer_sig_from_docstring(docstr: str, name: str) -> Optional[List[FunctionSi
with contextlib.suppress(tokenize.TokenError):
for token in tokenize.tokenize(io.BytesIO(docstr.encode('utf-8')).readline):
state.add_token(token)
return state.get_signatures()
ret = state.get_signatures()

def is_unique_args(sig: FunctionSig) -> bool:
"""return true if function argument names are unique"""
return len(sig.args) == len(set((x.name for x in sig.args)))
This conversation was marked as resolved by wiktorn

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 19, 2019

Member

x is a rather unimaginative variable name. How about arg?


# return only signatures, that have unique argument names. mypy fails on non-uqniue arg names
# TODO: emit a warning for duplicate argument names
return [x for x in ret if is_unique_args(x)]
This conversation was marked as resolved by wiktorn

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 19, 2019

Member

Ditto, how about sig? (Actually ret isn't such a great name either -- maybe sigs?)



def infer_arg_sig_from_docstring(docstr: str) -> List[ArgSig]:
@@ -167,14 +167,18 @@ def generate_c_function_stub(module: ModuleType,
for signature in inferred:
sig = []
for arg in signature.args:
if arg.name == self_var or not arg.type:
# no type
sig.append(arg.name)
if arg.name == self_var:
arg_def = self_var
else:
# type info
sig.append('{}: {}'.format(arg.name, strip_or_import(arg.type,
module,
imports)))
arg_def = arg.name

if arg.type:
arg_def += ": " + strip_or_import(arg.type, module, imports)

if arg.default:
arg_def += " = ..."

sig.append(arg_def)

if is_overloaded:
output.append('@overload')
@@ -218,6 +218,19 @@ def test_infer_sig_from_docstring(self) -> None:

assert_equal(infer_sig_from_docstring('\nfunc[(x: foo.bar, invalid]', 'func'), [])

assert_equal(infer_sig_from_docstring('\nfunc(x: invalid::type<with_template>)', 'func'),
[FunctionSig(name='func', args=[ArgSig(name='x', type=None)],
ret_type='Any')])

assert_equal(infer_sig_from_docstring('\nfunc(x: str="")', 'func'),
[FunctionSig(name='func', args=[ArgSig(name='x', type='str', default=True)],
ret_type='Any')])

def test_infer_sig_from_docstring_duplicate_args(self) -> None:
assert_equal(infer_sig_from_docstring('\nfunc(x, x) -> str\nfunc(x, y) -> int', 'func'),
This conversation was marked as resolved by wiktorn

This comment has been minimized.

Copy link
@gvanrossum

gvanrossum Feb 19, 2019

Member

I take it there's some kind of bug in pybind11 that sometimes generates func(x, x)?

This comment has been minimized.

Copy link
@wiktorn

wiktorn Feb 23, 2019

Author Contributor

There is a bug in pybind11 that sometimes generates signatures using C++ types instead of Python types. As I tried some sanitizations I got errors for duplicate arguments. Though right now those invalid signatures are just dropped, so this is not longer the case

[FunctionSig(name='func', args=[ArgSig(name='x'), ArgSig(name='y')],
ret_type='int')])

def test_infer_arg_sig_from_docstring(self) -> None:
assert_equal(infer_arg_sig_from_docstring("(*args, **kwargs)"),
[ArgSig(name='*args'), ArgSig(name='**kwargs')])
@@ -380,6 +393,21 @@ def test(self, arg0: str) -> None:
assert_equal(output, ['def test(self, arg0: int) -> Any: ...'])
assert_equal(imports, [])

def test_generate_c_type_with_docstring_empty_default(self) -> None:
class TestClass:
def test(self, arg0: str = "") -> None:
"""
test(self: TestClass, arg0: str = "")
"""
pass
output = [] # type: List[str]
imports = [] # type: List[str]
mod = ModuleType(TestClass.__module__, '')
generate_c_function_stub(mod, 'test', TestClass.test, output, imports,
self_var='self', class_name='TestClass')
assert_equal(output, ['def test(self, arg0: str = ...) -> Any: ...'])
assert_equal(imports, [])

def test_generate_c_function_other_module_arg(self) -> None:
"""Test that if argument references type from other module, module will be imported."""
# Provide different type in python spec than in docstring to make sure, that docstring
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.