From 4a89d0345f3d743778a3ad9ee375aa884332d861 Mon Sep 17 00:00:00 2001 From: Matthew Flamm Date: Tue, 20 Sep 2022 11:31:59 -0400 Subject: [PATCH 1/8] validate optional parameters --- numpydoc/tests/test_validate.py | 46 +++++++++++++++++++++++-- numpydoc/validate.py | 59 +++++++++++++++++++++++---------- 2 files changed, 85 insertions(+), 20 deletions(-) diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index 080affab..33ec9858 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -190,7 +190,7 @@ def head1(self, n=5): Parameters ---------- - n : int + n : int, default 5 Number of values to return. Returns @@ -224,7 +224,7 @@ def summary_starts_with_number(self, n=5): Parameters ---------- - n : int + n : int, default 5 4 Number of values to return. Returns @@ -506,6 +506,31 @@ def parameters_with_trailing_underscores(self, str_): """ pass + def optional_params(self, a=None, b=2, c="Thing 1"): + """ + Test different ways of testing optional parameters. + + There are three ways to document optional paramters. + + Parameters + ---------- + a : int, optional + Default is implicitly determined. + b : int, default 5 + Default is explicitly documented. + c : {"Thing 1", "Thing 2"} + Which thing. + + See Also + -------- + related : Something related. + + Examples + -------- + >>> result = 1 + 1 + """ + pass + class BadGenericDocStrings: """Everything here has a bad docstring""" @@ -922,6 +947,17 @@ def bad_parameter_spacing(self, a, b): """ pass + def no_documented_optional(self, a=5): + """ + Missing optional. + + Parameters + ---------- + a : int + Missing optional. + """ + pass + class BadReturns: def return_not_documented(self): @@ -1145,6 +1181,7 @@ def test_good_class(self, capsys): "warnings", "valid_options_in_parameter_description_sets", "parameters_with_trailing_underscores", + "optional_params", ], ) def test_good_functions(self, capsys, func): @@ -1357,6 +1394,11 @@ def test_bad_generic_functions(self, capsys, func): ("No error yet?",), marks=pytest.mark.xfail, ), + ( + "BadParameters", + "no_documented_optional", + ('Parameter "a" is optional but not documented',), + ), # Returns tests ("BadReturns", "return_not_documented", ("No Returns section found",)), ("BadReturns", "yield_not_documented", ("No Yields section found",)), diff --git a/numpydoc/validate.py b/numpydoc/validate.py index cc058f0d..06125d77 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -79,6 +79,7 @@ "PR09": 'Parameter "{param_name}" description should finish with "."', "PR10": 'Parameter "{param_name}" requires a space before the colon ' "separating the parameter name and type", + "PR11": 'Parameter "{param_name}" is optional but not documented', "RT01": "No Returns section found", "RT02": "The first line of the Returns section should contain only the " "type, unless multiple values are being returned", @@ -294,17 +295,6 @@ def doc_all_parameters(self): @property def signature_parameters(self): - def add_stars(param_name, info): - """ - 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 @@ -317,19 +307,46 @@ def add_stars(param_name, info): # Some objects, mainly in C extensions do not support introspection # of the signature return tuple() + params = dict(sig.parameters) + + if params: + first_param = next(iter(params.keys())) + if first_param in ("self", "cls"): + del params[first_param] - 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 + @staticmethod + def _add_stars(param_name, info): + """ + 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 + + @property + def signature_parameters_names(self): + return tuple( + self._add_stars(param, info) + for param, info in self.signature_parameters.items() + ) + + @property + def optional_signature_parameter_names(self): + return tuple( + self._add_stars(param, info) + for param, info in self.signature_parameters.items() + if info.default is not inspect._empty + ) + @property def parameter_mismatches(self): errs = [] - signature_params = self.signature_parameters + signature_params = self.signature_parameters_names all_params = tuple(param.replace("\\", "") for param in self.doc_all_parameters) missing = set(signature_params) - set(all_params) if missing: @@ -593,6 +610,12 @@ def validate(obj_name): wrong_type=wrong_type, ) ) + + for param in doc.optional_signature_parameter_names: + type = doc.parameter_type(param) + if "optional" not in type and "{" not in type and "default" not in type: + errs.append(error("PR11", param_name=param)) + errs.extend(_check_desc(kind_desc[1], "PR07", "PR08", "PR09", param_name=param)) if doc.is_function_or_method: From cf34a10302a9771b23e957675c8ac466b655cdc0 Mon Sep 17 00:00:00 2001 From: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com> Date: Tue, 20 Sep 2022 16:00:28 +0000 Subject: [PATCH 2/8] consistent naming --- numpydoc/validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/numpydoc/validate.py b/numpydoc/validate.py index 06125d77..55a21128 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -336,7 +336,7 @@ def signature_parameters_names(self): ) @property - def optional_signature_parameter_names(self): + def optional_signature_parameters_names(self): return tuple( self._add_stars(param, info) for param, info in self.signature_parameters.items() @@ -611,7 +611,7 @@ def validate(obj_name): ) ) - for param in doc.optional_signature_parameter_names: + for param in doc.optional_signature_parameters_names: type = doc.parameter_type(param) if "optional" not in type and "{" not in type and "default" not in type: errs.append(error("PR11", param_name=param)) From 4cd733cc7e03bcc08b8afce207da6065a062bc51 Mon Sep 17 00:00:00 2001 From: Matthew Flamm Date: Tue, 20 Sep 2022 13:48:39 -0400 Subject: [PATCH 3/8] fix incompatible return --- numpydoc/validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/numpydoc/validate.py b/numpydoc/validate.py index 06125d77..f939ef6a 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -300,13 +300,13 @@ def signature_parameters(self): self.name.split(".")[-1] in self.obj._accessors ): # accessor classes have a signature but don't want to show this - return tuple() + return dict() try: sig = inspect.signature(self.obj) except (TypeError, ValueError): # Some objects, mainly in C extensions do not support introspection # of the signature - return tuple() + return dict() params = dict(sig.parameters) if params: From 83c4830009362a3be42f1255286bfd0bb364023a Mon Sep 17 00:00:00 2001 From: Matthew Flamm Date: Tue, 20 Sep 2022 15:10:36 -0400 Subject: [PATCH 4/8] add inverse check and fix repeated check --- numpydoc/tests/test_validate.py | 25 +++++++++++++++++++++++-- numpydoc/validate.py | 24 ++++++++++++++++-------- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index 33ec9858..73e61182 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -949,7 +949,7 @@ def bad_parameter_spacing(self, a, b): def no_documented_optional(self, a=5): """ - Missing optional. + Missing optional in docstring. Parameters ---------- @@ -958,6 +958,19 @@ def no_documented_optional(self, a=5): """ pass + def no_default_when_documented_optional(self, a, b): + """ + Missing default in signature. + + Parameters + ---------- + a : int, optional + One way to denote optional. + b : int, default 5 + Another way to denote optional. + """ + pass + class BadReturns: def return_not_documented(self): @@ -1397,7 +1410,15 @@ def test_bad_generic_functions(self, capsys, func): ( "BadParameters", "no_documented_optional", - ('Parameter "a" is optional but not documented',), + ('Parameter "a" is optional but not documented, or vice versa',), + ), + ( + "BadParameters", + "no_default_when_documented_optional", + ( + 'Parameter "a" is optional but not documented, or vice versa', + 'Parameter "b" is optional but not documented, or vice versa', + ), ), # Returns tests ("BadReturns", "return_not_documented", ("No Returns section found",)), diff --git a/numpydoc/validate.py b/numpydoc/validate.py index 9b1f020c..d36490b6 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -79,7 +79,7 @@ "PR09": 'Parameter "{param_name}" description should finish with "."', "PR10": 'Parameter "{param_name}" requires a space before the colon ' "separating the parameter name and type", - "PR11": 'Parameter "{param_name}" is optional but not documented', + "PR11": 'Parameter "{param_name}" is optional but not documented, ' "or vice versa", "RT01": "No Returns section found", "RT02": "The first line of the Returns section should contain only the " "type, unless multiple values are being returned", @@ -583,7 +583,8 @@ def validate(obj_name): for param, kind_desc in doc.doc_all_parameters.items(): if not param.startswith("*"): # Check can ignore var / kwargs - if not doc.parameter_type(param): + param_type = doc.parameter_type(param) + if not param_type: if ":" in param: errs.append(error("PR10", param_name=param.split(":")[0])) else: @@ -593,13 +594,19 @@ def validate(obj_name): errs.append(error("PR05", param_name=param)) # skip common_type_error checks when the param type is a set of # options - if "{" in doc.parameter_type(param): + if "{" in param_type: continue common_type_errors = [ ("integer", "int"), ("boolean", "bool"), ("string", "str"), ] + + # check that documented optional param has default in sig + if "optional" in param_type or "default" in param_type: + if param not in doc.optional_signature_parameters_names: + errs.append(error("PR11", param_name=param)) + for wrong_type, right_type in common_type_errors: if wrong_type in doc.parameter_type(param): errs.append( @@ -611,13 +618,14 @@ def validate(obj_name): ) ) - for param in doc.optional_signature_parameters_names: - type = doc.parameter_type(param) - if "optional" not in type and "{" not in type and "default" not in type: - errs.append(error("PR11", param_name=param)) - errs.extend(_check_desc(kind_desc[1], "PR07", "PR08", "PR09", param_name=param)) + # check param with default in sig is documented as optional + for param in doc.optional_signature_parameters_names: + type = doc.parameter_type(param) + if "optional" not in type and "{" not in type and "default" not in type: + errs.append(error("PR11", param_name=param)) + if doc.is_function_or_method: if not doc.returns: if doc.method_returns_something: From 3ac46bfd3c324e5ce644a2bbe7a32fd6942c1ac8 Mon Sep 17 00:00:00 2001 From: Matthew Flamm Date: Thu, 22 Sep 2022 18:01:02 -0400 Subject: [PATCH 5/8] dont use builtin type --- numpydoc/validate.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/numpydoc/validate.py b/numpydoc/validate.py index d36490b6..be046e25 100644 --- a/numpydoc/validate.py +++ b/numpydoc/validate.py @@ -622,8 +622,12 @@ def validate(obj_name): # check param with default in sig is documented as optional for param in doc.optional_signature_parameters_names: - type = doc.parameter_type(param) - if "optional" not in type and "{" not in type and "default" not in type: + param_type = doc.parameter_type(param) + if ( + "optional" not in param_type + and "{" not in param_type + and "default" not in param_type + ): errs.append(error("PR11", param_name=param)) if doc.is_function_or_method: From 8cd766a8859becbce3dc80f94e2d0674f4586a1d Mon Sep 17 00:00:00 2001 From: Matthew Flamm Date: Wed, 12 Oct 2022 08:13:34 -0400 Subject: [PATCH 6/8] expand tests for None default --- numpydoc/tests/test_validate.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index 73e61182..048f99ab 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -958,6 +958,17 @@ def no_documented_optional(self, a=5): """ pass + def no_documented_optional_when_None(self, a=None): + """ + Missing optional in docstring when default is None. + + Parameters + ---------- + a : int + Missing optional. + """ + pass + def no_default_when_documented_optional(self, a, b): """ Missing default in signature. From af10af70dccb373c934fbca60b07b04759cba730 Mon Sep 17 00:00:00 2001 From: Matthew Flamm Date: Wed, 12 Oct 2022 08:20:46 -0400 Subject: [PATCH 7/8] add tests that distinguish between kwarg and default --- numpydoc/tests/test_validate.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index 048f99ab..c8299797 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -506,7 +506,7 @@ def parameters_with_trailing_underscores(self, str_): """ pass - def optional_params(self, a=None, b=2, c="Thing 1"): + def optional_params(self, *, not_optional, a=None, b=2, c="Thing 1"): """ Test different ways of testing optional parameters. @@ -514,6 +514,8 @@ def optional_params(self, a=None, b=2, c="Thing 1"): Parameters ---------- + not_optional : str + A keyword arg that is not optional. a : int, optional Default is implicitly determined. b : int, default 5 @@ -958,6 +960,18 @@ def no_documented_optional(self, a=5): """ pass + def documented_optional_but_kwarg(self, *, a): + """ + Missing optional in docstring. + + Parameters + ---------- + a : int, optional + Keyword arg mislabelled as optional when there is no + default. + """ + pass + def no_documented_optional_when_None(self, a=None): """ Missing optional in docstring when default is None. From 67b9eab7fb0cb00d4194f1998e1b7d765140979a Mon Sep 17 00:00:00 2001 From: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com> Date: Wed, 15 Mar 2023 13:25:52 -0400 Subject: [PATCH 8/8] Fix whitespace from conflict merge --- numpydoc/tests/test_validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/numpydoc/tests/test_validate.py b/numpydoc/tests/test_validate.py index c42e5aef..a5996f15 100644 --- a/numpydoc/tests/test_validate.py +++ b/numpydoc/tests/test_validate.py @@ -522,7 +522,7 @@ def optional_params(self, *, not_optional, a=None, b=2, c="Thing 1"): Default is explicitly documented. c : {"Thing 1", "Thing 2"} Which thing. - + See Also -------- related : Something related.