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

DOC: Fixing test and validate docstrings for section ordering #23245

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions scripts/tests/test_validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,37 @@ def head1(self, n=5):
"""
return self.iloc[:n]

def order(self):
"""
In this case, we are illustrating the correct ordering
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a example of good docstring , this summary should finish with a period and fit in one line. Tests should be failing because of it if this docstring is tested.

of sections in a docstring

Parameters
----------
pat : str
Pattern to check for within each element.
case : bool, default True
Whether check should be done with case sensitivity.
na : object, default np.nan
Fill value for missing data.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameters should be in the signature, tests should also be failing


Returns
-------
Series
Original series with boolean values.

Examples
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this validates that order of sections is coreect, we should have all sectiona, see also and notes, and proba ly a extended summary too

--------
>>> s = pd.Series(['Antelope', 'Lion', 'Zebra', np.nan])
>>> s.str.contains(pat='a')
0 False
1 False
2 True
3 NaN
dtype: object
"""
pass

def contains(self, pat, case=True, na=np.nan):
"""
Return whether each value contains `pat`.
Expand Down Expand Up @@ -502,6 +533,42 @@ def no_punctuation(self):
return "Hello world!"


class BadOrder(object):

def wrong_section_order(self):
"""
See also should not come before Returns section.

See Also
-------

Returns
-------
str
A nice greeting
"""

def wrong_section_order1(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to test all the sections we need to test every pair of contiguos section, parameters/return...

"""
Examples should not come before Parameters section.

Examples
-------
>>> s = pd.Series(['Antelope', 'Lion', 'Zebra', np.nan])
>>> s.str.contains(pat='a')
0 False
1 False
2 True
3 NaN
dtype: object

Parameters
-------
str
A nice greeting
"""


class TestValidator(object):

def _import_path(self, klass=None, func=None):
Expand Down Expand Up @@ -540,7 +607,7 @@ def test_good_class(self):
@capture_stderr
@pytest.mark.parametrize("func", [
'plot', 'sample', 'random_letters', 'sample_values', 'head', 'head1',
'contains', 'mode'])
'order', 'contains', 'mode'])
def test_good_functions(self, func):
errors = validate_one(self._import_path(
klass='GoodDocStrings', func=func))['errors']
Expand Down Expand Up @@ -600,7 +667,12 @@ def test_bad_generic_functions(self, func):
pytest.param('BadReturns', 'no_description', ('foo',),
marks=pytest.mark.xfail),
pytest.param('BadReturns', 'no_punctuation', ('foo',),
marks=pytest.mark.xfail)
marks=pytest.mark.xfail),
# Section ordering tests
('BadOrder', 'wrong_section_order',
('Section See Also should not be before Return section',)),
('BadOrder', 'wrong_section_order1',
('Examples section should not be before Parameters',))
])
def test_bad_examples(self, capsys, klass, func, msgs):
result = validate_one(self._import_path(klass=klass, func=func)) # noqa:F821
Expand Down
16 changes: 16 additions & 0 deletions scripts/validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,18 @@ def deprecated(self):
def mentioned_private_classes(self):
return [klass for klass in PRIVATE_CLASSES if klass in self.raw_doc]

@property
def section_order(self):
sections = ["Summary", "Parameters", "Returns",
"Yields", "See Also", "Notes", "Examples"]
section_index = []
for section in sections:
if section not in self.raw_doc:
continue
else:
section_index.append(self.raw_doc.index(section))
return section_index
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this implementation doesn't seem realiable, if the summary contains for example Returns... this will return the wrong indices, right?

Also, the name of the propertyis not very intuitive on what it does, and if something like this needs to be implemented, we should have tests for it.

I think we use numpydoc for parsing the docstring. May be there is something there that can be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datapythonista yup that’s true - I wanted to use numpydoc for parsing but wasn’t sure it would work for the summary and short summary sections as there is usually no title/header to parse the content.... trying to find a workaround


@property
def examples_errors(self):
flags = doctest.NORMALIZE_WHITESPACE | doctest.IGNORE_EXCEPTION_DETAIL
Expand Down Expand Up @@ -492,6 +504,10 @@ def validate_one(func_name):
errs.append('Private classes ({}) should not be mentioned in public '
'docstring.'.format(mentioned_errs))

section_index = doc.section_order
if sorted(section_index) != section_index:
errs.append("The section titles are not in the correct order.")

if not doc.see_also:
wrns.append('See Also section not found')
else:
Expand Down