From 79e59f43c2937c736b22cc9a135057c245c96be4 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Wed, 2 Oct 2019 17:03:10 -0400 Subject: [PATCH 01/16] Use signature instead of getfullargspec Using signature allows the validation to get to the root function when a function is decorated. This reduced a significant amount of the errors in the string.py file --- scripts/validate_docstrings.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 401eaf8ff5ed5..22f5eab2feae3 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -436,16 +436,20 @@ def signature_parameters(self): # accessor classes have a signature but don't want to show this return tuple() try: - sig = inspect.getfullargspec(self.obj) + sig = inspect.signature(self.obj) except (TypeError, ValueError): # Some objects, mainly in C extensions do not support introspection # of the signature return tuple() - params = sig.args - if sig.varargs: - params.append("*" + sig.varargs) - if sig.varkw: - params.append("**" + sig.varkw) + params = list() + for parameter, data in sig.parameters.items(): + if data.kind == inspect.Parameter.VAR_POSITIONAL: + params.append("*" + parameter) + elif data.kind == inspect.Parameter.VAR_KEYWORD: + params.append("**" + parameter) + else: + params.append(parameter) + params = tuple(params) if params and params[0] in ("self", "cls"): return params[1:] From d65e26c5465d51656b99de882c20c665021b7c64 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Wed, 2 Oct 2019 23:28:41 -0400 Subject: [PATCH 02/16] Account for args and kwargs on same line Check for variations on "*args, **kwargs" while loading in the documentation parameters, and load them in separately if found. --- scripts/validate_docstrings.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 22f5eab2feae3..bc6651f7469d3 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -422,10 +422,17 @@ def needs_summary(self): @property def doc_parameters(self): - return collections.OrderedDict( - (name, (type_, "".join(desc))) - for name, type_, desc in self.doc["Parameters"] - ) + var_arg_combinations = {"*args, **kwargs", "*args, **kwds", "**kwargs, *args", "**kwds, *args"} + docs = collections.OrderedDict() + for name, type_, desc in self.doc["Parameters"]: + info = (type_, desc) + if name in var_arg_combinations: + args = name.split(", ") + docs[args[0]] = info + docs[args[1]] = info + else: + docs[name] = info + return docs @property def signature_parameters(self): From 9bff615ab15202413fd53b706ef0c307ee8c7fd2 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Thu, 3 Oct 2019 00:03:02 -0400 Subject: [PATCH 03/16] Styling Changes Pep8 conformity --- scripts/validate_docstrings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index bc6651f7469d3..06dee007f7b33 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -422,7 +422,8 @@ def needs_summary(self): @property def doc_parameters(self): - var_arg_combinations = {"*args, **kwargs", "*args, **kwds", "**kwargs, *args", "**kwds, *args"} + var_arg_combinations = {"*args, **kwargs", "*args, **kwds", + "**kwargs, *args", "**kwds, *args"} docs = collections.OrderedDict() for name, type_, desc in self.doc["Parameters"]: info = (type_, desc) From 3814799606a9ae07b679af19a83ade8d8f3f824d Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Wed, 2 Oct 2019 17:03:10 -0400 Subject: [PATCH 04/16] BUG: Use signature instead of getfullargspec Using signature allows the validation to get to the root function when a function is decorated. This reduced a significant amount of the errors in the string.py file --- scripts/validate_docstrings.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 401eaf8ff5ed5..22f5eab2feae3 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -436,16 +436,20 @@ def signature_parameters(self): # accessor classes have a signature but don't want to show this return tuple() try: - sig = inspect.getfullargspec(self.obj) + sig = inspect.signature(self.obj) except (TypeError, ValueError): # Some objects, mainly in C extensions do not support introspection # of the signature return tuple() - params = sig.args - if sig.varargs: - params.append("*" + sig.varargs) - if sig.varkw: - params.append("**" + sig.varkw) + params = list() + for parameter, data in sig.parameters.items(): + if data.kind == inspect.Parameter.VAR_POSITIONAL: + params.append("*" + parameter) + elif data.kind == inspect.Parameter.VAR_KEYWORD: + params.append("**" + parameter) + else: + params.append(parameter) + params = tuple(params) if params and params[0] in ("self", "cls"): return params[1:] From 271defc93103fbbd520bcd8fb9fa32ac40c2caec Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Wed, 2 Oct 2019 23:28:41 -0400 Subject: [PATCH 05/16] BUG: Account for args and kwargs on same line Check for variations on "*args, **kwargs" while loading in the documentation parameters, and load them in separately if found. --- scripts/validate_docstrings.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 22f5eab2feae3..bc6651f7469d3 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -422,10 +422,17 @@ def needs_summary(self): @property def doc_parameters(self): - return collections.OrderedDict( - (name, (type_, "".join(desc))) - for name, type_, desc in self.doc["Parameters"] - ) + var_arg_combinations = {"*args, **kwargs", "*args, **kwds", "**kwargs, *args", "**kwds, *args"} + docs = collections.OrderedDict() + for name, type_, desc in self.doc["Parameters"]: + info = (type_, desc) + if name in var_arg_combinations: + args = name.split(", ") + docs[args[0]] = info + docs[args[1]] = info + else: + docs[name] = info + return docs @property def signature_parameters(self): From 7095b99f837043a5563702c59a51fb5f7b755fe4 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Thu, 3 Oct 2019 00:03:02 -0400 Subject: [PATCH 06/16] Styling Changes Pep8 conformity --- scripts/validate_docstrings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index bc6651f7469d3..06dee007f7b33 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -422,7 +422,8 @@ def needs_summary(self): @property def doc_parameters(self): - var_arg_combinations = {"*args, **kwargs", "*args, **kwds", "**kwargs, *args", "**kwds, *args"} + var_arg_combinations = {"*args, **kwargs", "*args, **kwds", + "**kwargs, *args", "**kwds, *args"} docs = collections.OrderedDict() for name, type_, desc in self.doc["Parameters"]: info = (type_, desc) From 294b11651f370bea3f9a3bfaa1333dc13a370899 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Thu, 3 Oct 2019 00:50:17 -0400 Subject: [PATCH 07/16] DOC: Idxmin/max fixed Those functions cause issues during the testing. This has most likely been covered by some PR merged already. --- pandas/core/series.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pandas/core/series.py b/pandas/core/series.py index c87e371354f63..6b9b08457c778 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -2076,12 +2076,12 @@ def idxmin(self, axis=0, skipna=True, *args, **kwargs): Parameters ---------- - skipna : bool, default True - Exclude NA/null values. If the entire Series is NA, the result - will be NA. axis : int, default 0 For compatibility with DataFrame.idxmin. Redundant for application on Series. + skipna : bool, default True + Exclude NA/null values. If the entire Series is NA, the result + will be NA. *args, **kwargs Additional keywords have no effect but might be accepted for compatibility with NumPy. @@ -2146,12 +2146,12 @@ def idxmax(self, axis=0, skipna=True, *args, **kwargs): Parameters ---------- - skipna : bool, default True - Exclude NA/null values. If the entire Series is NA, the result - will be NA. axis : int, default 0 For compatibility with DataFrame.idxmax. Redundant for application on Series. + skipna : bool, default True + Exclude NA/null values. If the entire Series is NA, the result + will be NA. *args, **kwargs Additional keywords have no effect but might be accepted for compatibility with NumPy. From adc77e8b0bc1c81444d2398adb3b1fe200cba874 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Thu, 3 Oct 2019 11:43:06 -0400 Subject: [PATCH 08/16] BUG: Fixed desc being loaded as list instead of str Desc was being loaded in without being turned into a string. This was causing issues when indexes were used to check capitalization and the final period. --- scripts/validate_docstrings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 06dee007f7b33..93cebcde26ad7 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -426,7 +426,7 @@ def doc_parameters(self): "**kwargs, *args", "**kwds, *args"} docs = collections.OrderedDict() for name, type_, desc in self.doc["Parameters"]: - info = (type_, desc) + info = (type_, "".join(desc)) if name in var_arg_combinations: args = name.split(", ") docs[args[0]] = info From 877ebb36c2c0f09e1768338d31968cf71ba2d02d Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Thu, 3 Oct 2019 12:12:12 -0400 Subject: [PATCH 09/16] CLN: Missed styling changes For some reason black didn't catch the validation script the first time. It should be fixed now. --- scripts/validate_docstrings.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 93cebcde26ad7..25e1099c7b7b8 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -422,8 +422,12 @@ def needs_summary(self): @property def doc_parameters(self): - var_arg_combinations = {"*args, **kwargs", "*args, **kwds", - "**kwargs, *args", "**kwds, *args"} + var_arg_combinations = { + "*args, **kwargs", + "*args, **kwds", + "**kwargs, *args", + "**kwds, *args", + } docs = collections.OrderedDict() for name, type_, desc in self.doc["Parameters"]: info = (type_, "".join(desc)) From 5a13341b5c09fa4a2e8725f95929d311bbe4eec3 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Fri, 4 Oct 2019 00:44:25 -0400 Subject: [PATCH 10/16] TST: Added tests for decorated functions Tests added to test_validate_docstrings that check that it handles decorated functions as predicted. --- scripts/tests/test_validate_docstrings.py | 64 +++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index 85e5bf239cbfa..947877abc96df 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -1,3 +1,4 @@ +from functools import wraps import io import random import string @@ -12,6 +13,39 @@ validate_one = validate_docstrings.validate_one +class Decorators: + @staticmethod + def good_decorator(func): + @wraps(func) + def wrapper(*args, **kwargs): + """ + Wrapper function. + + This docstring should be hidden, and not used during validation. + Using the @wraps decorator should allow validation to get the + signature of the wrapped function when comparing against the + docstring. + """ + return func(*args, **kwargs) + + return wrapper + + @staticmethod + def missing_wraps(func): + def wrapper(*args): + """ + Wrapper function. + + The signature of this wrapper should override the underlying + function. So, we should see errors regarding the parameters, + like expecting documentation for *args and **kwargs and missing + documentation for the underlying signature. + """ + return func(*args) + + return wrapper + + class GoodDocStrings: """ Collection of good doc strings. @@ -53,6 +87,21 @@ def sample(self): """ return random.random() + @Decorators.good_decorator + def decorated_sample(self): + """ + Generate and return a random number. + + The value is sampled from a continuous uniform distribution between + 0 and 1. + + Returns + ------- + float + Random number generated. + """ + return random.random() + def random_letters(self): """ Generate and return a sequence of random letters. @@ -634,6 +683,19 @@ def list_incorrect_parameter_type(self, kind): """ pass + @Decorators.missing_wraps + def bad_decorator(self, kind: bool): + """ + The decorator is missing the @wraps, and overrides the signature + of this method. + + Parameters + ---------- + kind : bool + Foo bar baz + """ + pass + class BadReturns: def return_not_documented(self): @@ -828,6 +890,7 @@ def test_good_class(self, capsys): [ "plot", "sample", + "decorated_sample", "random_letters", "sample_values", "head", @@ -1002,6 +1065,7 @@ def test_bad_generic_functions(self, capsys, func): "list_incorrect_parameter_type", ('Parameter "kind" type should use "str" instead of "string"',), ), + ("BadParameters", "bad_decorator", ("Parameters {*args} not documented",)), pytest.param( "BadParameters", "blank_lines", From 7d420be2168be7b72626dc741ecd5d4aab2e21c7 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Fri, 4 Oct 2019 00:46:00 -0400 Subject: [PATCH 11/16] CLN: Missed whitespace changes Fixed trailing whitespace on two lines. --- scripts/tests/test_validate_docstrings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index 947877abc96df..daeb53cc16ad1 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -22,8 +22,8 @@ def wrapper(*args, **kwargs): Wrapper function. This docstring should be hidden, and not used during validation. - Using the @wraps decorator should allow validation to get the - signature of the wrapped function when comparing against the + Using the @wraps decorator should allow validation to get the + signature of the wrapped function when comparing against the docstring. """ return func(*args, **kwargs) From 17176c20db9930edc571f2ec2ec3de71c3d87709 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Sun, 6 Oct 2019 23:44:55 -0400 Subject: [PATCH 12/16] TST: Revert change to arg handling Reverting back to master's version so the issue can be addressed in another PR. --- scripts/validate_docstrings.py | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 25e1099c7b7b8..22f5eab2feae3 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -422,22 +422,10 @@ def needs_summary(self): @property def doc_parameters(self): - var_arg_combinations = { - "*args, **kwargs", - "*args, **kwds", - "**kwargs, *args", - "**kwds, *args", - } - docs = collections.OrderedDict() - for name, type_, desc in self.doc["Parameters"]: - info = (type_, "".join(desc)) - if name in var_arg_combinations: - args = name.split(", ") - docs[args[0]] = info - docs[args[1]] = info - else: - docs[name] = info - return docs + return collections.OrderedDict( + (name, (type_, "".join(desc))) + for name, type_, desc in self.doc["Parameters"] + ) @property def signature_parameters(self): From b1b7a34d1223618d813d19e1d3e591cad4551af7 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Mon, 7 Oct 2019 20:31:23 -0400 Subject: [PATCH 13/16] Better test and readability improvement I added a parameter to the test so that it's more visible, and changed the validation script to use a generator straight into a tuple. I also switched off of using `items`, since it's a set and therefore might give different orders for the parameters. --- scripts/tests/test_validate_docstrings.py | 14 ++++++++------ scripts/validate_docstrings.py | 21 ++++++++++++--------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index daeb53cc16ad1..58eba72f03798 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -88,19 +88,21 @@ def sample(self): return random.random() @Decorators.good_decorator - def decorated_sample(self): + def decorated_sample(self, max): """ - Generate and return a random number. + Generate and return a random integer between 0 and max. - The value is sampled from a continuous uniform distribution between - 0 and 1. + Parameters + ---------- + max : int + The maximum value of the random number. Returns ------- - float + int Random number generated. """ - return random.random() + return random.randint(0, max) def random_letters(self): """ diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 22f5eab2feae3..c776b8109df3c 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -429,6 +429,17 @@ def doc_parameters(self): @property def signature_parameters(self): + def add_stars(param_name :str, info: inspect.Parameter): + """ + Add stars to *args and **kwargs parameters + """ + if info.kind == inspect.Parameter.VAR_POSITIONAL: + return f"*{param_name}" + elif info.kind == inspect.Parameter.VAR_KEYWORD: + return f"**{param_name}" + else: + return param_name + if inspect.isclass(self.obj): if hasattr(self.obj, "_accessors") and ( self.name.split(".")[-1] in self.obj._accessors @@ -441,16 +452,8 @@ def signature_parameters(self): # Some objects, mainly in C extensions do not support introspection # of the signature return tuple() - params = list() - for parameter, data in sig.parameters.items(): - if data.kind == inspect.Parameter.VAR_POSITIONAL: - params.append("*" + parameter) - elif data.kind == inspect.Parameter.VAR_KEYWORD: - params.append("**" + parameter) - else: - params.append(parameter) - params = tuple(params) + params = tuple(add_stars(parameter, sig.parameters[parameter]) for parameter in sig.parameters) if params and params[0] in ("self", "cls"): return params[1:] return params From 5a236f3e4aaf443b4fc6ec1b1a4fc4410c7fb221 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Mon, 7 Oct 2019 20:42:42 -0400 Subject: [PATCH 14/16] Fixed PEP8 Requirements Fixed line length and whitespace requirements --- scripts/validate_docstrings.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index c776b8109df3c..db8fc2d8fc27d 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -429,7 +429,7 @@ def doc_parameters(self): @property def signature_parameters(self): - def add_stars(param_name :str, info: inspect.Parameter): + def add_stars(param_name: str, info: inspect.Parameter): """ Add stars to *args and **kwargs parameters """ @@ -453,7 +453,10 @@ def add_stars(param_name :str, info: inspect.Parameter): # of the signature return tuple() - params = tuple(add_stars(parameter, sig.parameters[parameter]) for parameter in sig.parameters) + params = tuple( + add_stars(parameter, sig.parameters[parameter]) + for parameter in sig.parameters + ) if params and params[0] in ("self", "cls"): return params[1:] return params From 449d5d19c7d391021e54aa647cfa63bc64e590d4 Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Sat, 12 Oct 2019 21:13:11 -0400 Subject: [PATCH 15/16] Remove bad parameter test Remove a test that doesn't quite examine the error we'd like. --- scripts/tests/test_validate_docstrings.py | 29 ----------------------- 1 file changed, 29 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index 0895881dd827c..fdde6db0b2300 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -30,21 +30,6 @@ def wrapper(*args, **kwargs): return wrapper - @staticmethod - def missing_wraps(func): - def wrapper(*args): - """ - Wrapper function. - - The signature of this wrapper should override the underlying - function. So, we should see errors regarding the parameters, - like expecting documentation for *args and **kwargs and missing - documentation for the underlying signature. - """ - return func(*args) - - return wrapper - class GoodDocStrings: """ @@ -715,19 +700,6 @@ def list_incorrect_parameter_type(self, kind): """ pass - @Decorators.missing_wraps - def bad_decorator(self, kind: bool): - """ - The decorator is missing the @wraps, and overrides the signature - of the method. - - Parameters - ---------- - kind : bool - Foo bar baz - """ - pass - def bad_parameter_spacing(self, a, b): """ The parameters on the same line have an extra space between them. @@ -1110,7 +1082,6 @@ def test_bad_generic_functions(self, capsys, func): "list_incorrect_parameter_type", ('Parameter "kind" type should use "str" instead of "string"',), ), - ("BadParameters", "bad_decorator", ("Parameters {*args} not documented",)), ( "BadParameters", "bad_parameter_spacing", From c9bdb28343a9218e2334f5478d3660bec4f165bd Mon Sep 17 00:00:00 2001 From: Nathan Abel Date: Sun, 13 Oct 2019 15:10:37 -0400 Subject: [PATCH 16/16] Use lru_cache instead of custom decorator Switched to using a builtin decorator --- scripts/tests/test_validate_docstrings.py | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index fdde6db0b2300..1506acc95edf9 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -1,4 +1,4 @@ -from functools import wraps +import functools import io import random import string @@ -13,24 +13,6 @@ validate_one = validate_docstrings.validate_one -class Decorators: - @staticmethod - def good_decorator(func): - @wraps(func) - def wrapper(*args, **kwargs): - """ - Wrapper function. - - This docstring should be hidden, and not used during validation. - Using the @wraps decorator should allow validation to get the - signature of the wrapped function when comparing against the - docstring. - """ - return func(*args, **kwargs) - - return wrapper - - class GoodDocStrings: """ Collection of good doc strings. @@ -87,7 +69,7 @@ def sample(self): """ return random.random() - @Decorators.good_decorator + @functools.lru_cache(None) def decorated_sample(self, max): """ Generate and return a random integer between 0 and max.