From e0f9689235cd1b1ecfbb086f844d49f6c8a140b9 Mon Sep 17 00:00:00 2001 From: igorfassen Date: Wed, 31 Oct 2018 17:43:28 +0100 Subject: [PATCH 01/10] DOC: add checks on the returns section in the docstrings (#23138) --- scripts/tests/test_validate_docstrings.py | 40 ++++++++++++++++++++--- scripts/validate_docstrings.py | 30 +++++++++++++++-- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index 0e10265a7291d..497a8095856b6 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -545,6 +545,30 @@ def no_punctuation(self): """ return "Hello world!" + def named_single_return(self): + """ + Provides name but returns only one value. + + Returns + ------- + s : str + A nice greeting. + """ + return "Hello world!" + + def no_capitalization(self): + """ + Forgets capitalization in return values descriptions. + + Returns + ------- + foo : str + the first returned string. + bar : str + the second returned string. + """ + return "Hello", "World!" + class BadSeeAlso(object): @@ -696,10 +720,18 @@ def test_bad_generic_functions(self, func): ('BadReturns', 'yield_not_documented', ('No Yields section found',)), pytest.param('BadReturns', 'no_type', ('foo',), marks=pytest.mark.xfail), - pytest.param('BadReturns', 'no_description', ('foo',), - marks=pytest.mark.xfail), - pytest.param('BadReturns', 'no_punctuation', ('foo',), - marks=pytest.mark.xfail), + ('BadReturns', 'no_description', + ('Return value has no description',)), + ('BadReturns', 'no_punctuation', + ('Return value description should finish with "."',)), + ('BadReturns', 'named_single_return', + ('No name is to be provided when returning a single value',)), + ('BadReturns', 'no_capitalization', + ('Return value "foo" description should start with a capital ' + 'letter',)), + ('BadReturns', 'no_capitalization', + ('Return value "bar" description should start with a capital ' + 'letter',)), # See Also tests ('BadSeeAlso', 'prefix_pandas', ('pandas.Series.rename in `See Also` section ' diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 29d485550be40..04a18c1f7a153 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -491,8 +491,34 @@ def validate_one(func_name): errs.append('\t{}'.format(param_err)) if doc.is_function_or_method: - if not doc.returns and "return" in doc.method_source: - errs.append('No Returns section found') + if not doc.returns: + if "return" in doc.method_source: + errs.append('No Returns section found') + else: + returns_errs = [] + if len(doc.returns) == 1 and doc.returns[0][1]: + returns_errs.append('No name is to be provided when ' + 'returning a single value.') + for name, type_, desc in doc.returns: + desc = ''.join(desc) + name = '"' + name + '" ' if type_ else '' + if not desc: + returns_errs.append('Return value {}has no ' + 'description'.format(name)) + else: + if not desc[0].isupper(): + returns_errs.append('Return value {}description ' + 'should start with a capital ' + 'letter'.format(name)) + if desc[-1] != '.': + returns_errs.append('Return value {}description ' + 'should finish with ' + '"."'.format(name)) + if returns_errs: + errs.append('Errors in returns section') + for returns_err in returns_errs: + errs.append('\t{}'.format(returns_err)) + if not doc.yields and "yield" in doc.method_source: errs.append('No Yields section found') From 20b32e7671b1b748012d0e1790f9fbe3a9dc8bc7 Mon Sep 17 00:00:00 2001 From: Marc Garcia Date: Mon, 5 Nov 2018 13:57:18 +0100 Subject: [PATCH 02/10] Update scripts/validate_docstrings.py Co-Authored-By: igorfassen --- 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 04a18c1f7a153..47ed68a428453 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -515,7 +515,7 @@ def validate_one(func_name): 'should finish with ' '"."'.format(name)) if returns_errs: - errs.append('Errors in returns section') + errs.append('Errors in Returns section') for returns_err in returns_errs: errs.append('\t{}'.format(returns_err)) From f2d6449d60b78fa9277049f553c01da28dfb5c4a Mon Sep 17 00:00:00 2001 From: igorfassen Date: Mon, 5 Nov 2018 14:03:27 +0100 Subject: [PATCH 03/10] update validate_docstrings.py: clearer error message --- scripts/validate_docstrings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 47ed68a428453..234ddbe6e116a 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -497,8 +497,9 @@ def validate_one(func_name): else: returns_errs = [] if len(doc.returns) == 1 and doc.returns[0][1]: - returns_errs.append('No name is to be provided when ' - 'returning a single value.') + returns_errs.append('The first line of the Returns section ' + 'should contain only the type, unless ' + 'multiple values are being returned.') for name, type_, desc in doc.returns: desc = ''.join(desc) name = '"' + name + '" ' if type_ else '' From 0d34a8805c17681b1c1f614e175c8fe1518a27b7 Mon Sep 17 00:00:00 2001 From: igorfassen Date: Mon, 5 Nov 2018 14:07:59 +0100 Subject: [PATCH 04/10] update test_validate_docstrings.py: fix expected error message --- scripts/tests/test_validate_docstrings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index 497a8095856b6..d28e34281cd34 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -725,7 +725,8 @@ def test_bad_generic_functions(self, func): ('BadReturns', 'no_punctuation', ('Return value description should finish with "."',)), ('BadReturns', 'named_single_return', - ('No name is to be provided when returning a single value',)), + ('The first line of the Returns section should contain only the ' + 'type, unless multiple values are being returned.',)), ('BadReturns', 'no_capitalization', ('Return value "foo" description should start with a capital ' 'letter',)), From b09c322e90e1b7ce7e5a00f6ef391ac48a08adac Mon Sep 17 00:00:00 2001 From: igorfassen Date: Mon, 5 Nov 2018 23:31:58 +0100 Subject: [PATCH 05/10] update validate_docstrings.py: Returns section validation * removed value name from error messages * updated the associated test case --- scripts/tests/test_validate_docstrings.py | 7 ++----- scripts/validate_docstrings.py | 25 +++++++++++------------ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index d28e34281cd34..a9e6101499909 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -563,7 +563,7 @@ def no_capitalization(self): Returns ------- foo : str - the first returned string. + The first returned string. bar : str the second returned string. """ @@ -728,10 +728,7 @@ def test_bad_generic_functions(self, func): ('The first line of the Returns section should contain only the ' 'type, unless multiple values are being returned.',)), ('BadReturns', 'no_capitalization', - ('Return value "foo" description should start with a capital ' - 'letter',)), - ('BadReturns', 'no_capitalization', - ('Return value "bar" description should start with a capital ' + ('Return value description should start with a capital ' 'letter',)), # See Also tests ('BadSeeAlso', 'prefix_pandas', diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 234ddbe6e116a..e694d6bdb95d1 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -500,21 +500,20 @@ def validate_one(func_name): returns_errs.append('The first line of the Returns section ' 'should contain only the type, unless ' 'multiple values are being returned.') + missing_desc, missing_cap, missing_period = False, False, False for name, type_, desc in doc.returns: desc = ''.join(desc) - name = '"' + name + '" ' if type_ else '' - if not desc: - returns_errs.append('Return value {}has no ' - 'description'.format(name)) - else: - if not desc[0].isupper(): - returns_errs.append('Return value {}description ' - 'should start with a capital ' - 'letter'.format(name)) - if desc[-1] != '.': - returns_errs.append('Return value {}description ' - 'should finish with ' - '"."'.format(name)) + missing_desc = missing_desc or not desc + missing_cap = missing_cap or desc and not desc[0].isupper() + missing_period = missing_period or desc and not desc.endswith('.') + if missing_desc: + returns_errs.append('Return value has no description.') + if missing_cap: + returns_errs.append('Return value description should start ' + 'with a capital letter.') + if missing_period: + returns_errs.append('Return value description should finish ' + 'with ".".') if returns_errs: errs.append('Errors in Returns section') for returns_err in returns_errs: From a1384d42a3eecb9e9cec33b6d93fbdb95831620a Mon Sep 17 00:00:00 2001 From: igorfassen Date: Tue, 6 Nov 2018 10:48:21 +0100 Subject: [PATCH 06/10] update validate_docstrings.py: split line to comply with pep 8 --- 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 340fdb5422c38..0314664580bf3 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -541,7 +541,8 @@ def validate_one(func_name): desc = ''.join(desc) missing_desc = missing_desc or not desc missing_cap = missing_cap or desc and not desc[0].isupper() - missing_period = missing_period or desc and not desc.endswith('.') + missing_period = (missing_period + or desc and not desc.endswith('.')) if missing_desc: returns_errs.append('Return value has no description.') if missing_cap: From 2f3f5bf04dc7f75995934130826d1d3a8285f560 Mon Sep 17 00:00:00 2001 From: igorfassen Date: Wed, 7 Nov 2018 08:57:50 +0100 Subject: [PATCH 07/10] update validate_docstrings.py: small fixes in Returns validation * removed the message `Errors in Returns section` * simpler variable initialization --- scripts/validate_docstrings.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 0314664580bf3..4b1dbc62a49d2 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -531,12 +531,11 @@ def validate_one(func_name): if "return" in doc.method_source: errs.append('No Returns section found') else: - returns_errs = [] if len(doc.returns) == 1 and doc.returns[0][1]: - returns_errs.append('The first line of the Returns section ' - 'should contain only the type, unless ' - 'multiple values are being returned.') - missing_desc, missing_cap, missing_period = False, False, False + errs.append('The first line of the Returns section ' + 'should contain only the type, unless ' + 'multiple values are being returned.') + missing_desc = missing_cap = missing_period = False for name, type_, desc in doc.returns: desc = ''.join(desc) missing_desc = missing_desc or not desc @@ -544,17 +543,12 @@ def validate_one(func_name): missing_period = (missing_period or desc and not desc.endswith('.')) if missing_desc: - returns_errs.append('Return value has no description.') + errs.append('Return value has no description.') if missing_cap: - returns_errs.append('Return value description should start ' - 'with a capital letter.') + errs.append('Return value description should start ' + 'with a capital letter.') if missing_period: - returns_errs.append('Return value description should finish ' - 'with ".".') - if returns_errs: - errs.append('Errors in Returns section') - for returns_err in returns_errs: - errs.append('\t{}'.format(returns_err)) + errs.append('Return value description should finish with ".".') if not doc.yields and "yield" in doc.method_source: errs.append('No Yields section found') From fdad765754eb5c366d2291a05b00a8f1b05b7978 Mon Sep 17 00:00:00 2001 From: igorfassen Date: Thu, 8 Nov 2018 22:10:06 +0100 Subject: [PATCH 08/10] update validate_docstrings.py: simplify returns section validation --- scripts/validate_docstrings.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index ea615c47fe23a..3e26ff9987205 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -627,19 +627,15 @@ def validate_one(func_name): else: if len(doc.returns) == 1 and doc.returns[0][1]: errs.append(error('RT02')) - missing_desc = missing_cap = missing_period = False for name, type_, desc in doc.returns: - desc = ''.join(desc) - missing_desc = missing_desc or not desc - missing_cap = missing_cap or desc and not desc[0].isupper() - missing_period = (missing_period - or desc and not desc.endswith('.')) - if missing_desc: - errs.append(error('RT03')) - if missing_cap: - errs.append(error('RT04')) - if missing_period: - errs.append(error('RT05')) + if not desc: + errs.append(error('RT03')) + else: + desc = ' '.join(desc) + if not desc[0].isupper(): + errs.append(error('RT04')) + if not desc.endswith('.'): + errs.append(error('RT05')) if not doc.yields and 'yield' in doc.method_source: errs.append(error('YD01')) From 58a0a91f503d007aff72a5b9bfc497c7c8b5fad3 Mon Sep 17 00:00:00 2001 From: igorfassen Date: Fri, 9 Nov 2018 11:30:23 +0100 Subject: [PATCH 09/10] update validate_docstrings.py: replace " by ' for homogenization --- 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 3e26ff9987205..ef42246675535 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -622,7 +622,7 @@ def validate_one(func_name): if doc.is_function_or_method: if not doc.returns: - if "return" in doc.method_source: + if 'return' in doc.method_source: errs.append(error('RT01')) else: if len(doc.returns) == 1 and doc.returns[0][1]: From 5ea78816de0d1fd4c2e2757474ccaa347e4c983c Mon Sep 17 00:00:00 2001 From: Marc Garcia Date: Sun, 30 Dec 2018 20:59:07 +0000 Subject: [PATCH 10/10] Minor fixes --- scripts/tests/test_validate_docstrings.py | 17 ++++++++++++++++- scripts/validate_docstrings.py | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/scripts/tests/test_validate_docstrings.py b/scripts/tests/test_validate_docstrings.py index 04fe41493fe0f..bb58449843096 100644 --- a/scripts/tests/test_validate_docstrings.py +++ b/scripts/tests/test_validate_docstrings.py @@ -646,7 +646,7 @@ def named_single_return(self): def no_capitalization(self): """ - Forgets capitalization in return values descriptions. + Forgets capitalization in return values description. Returns ------- @@ -657,6 +657,19 @@ def no_capitalization(self): """ return "Hello", "World!" + def no_period_multi(self): + """ + Forgets period in return values description. + + Returns + ------- + foo : str + The first returned string + bar : str + The second returned string. + """ + return "Hello", "World!" + class BadSeeAlso(object): @@ -863,6 +876,8 @@ def test_bad_generic_functions(self, capsys, func): ('BadReturns', 'no_capitalization', ('Return value description should start with a capital ' 'letter',)), + ('BadReturns', 'no_period_multi', + ('Return value description should finish with "."',)), # Examples tests ('BadGenericDocStrings', 'method', ('Do not import numpy, as it is imported automatically',)), diff --git a/scripts/validate_docstrings.py b/scripts/validate_docstrings.py index 060aa4718950f..4e389aed2b0d2 100755 --- a/scripts/validate_docstrings.py +++ b/scripts/validate_docstrings.py @@ -696,7 +696,7 @@ def get_validation_data(doc): else: if len(doc.returns) == 1 and doc.returns[0][1]: errs.append(error('RT02')) - for name, type_, desc in doc.returns: + for name_or_type, type_, desc in doc.returns: if not desc: errs.append(error('RT03')) else: