Skip to content
46 changes: 45 additions & 1 deletion scripts/tests/test_validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,18 @@ def mode(self, axis, numeric_only):
"""
pass

def good_import(self):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def good_import(self):
def good_imports(self):

"""
Ensure import other than numpy and pandas are fine.

Examples
--------
This example does not import pandas or import numpy.
>>> import time
>>> import datetime
"""
pass


class BadGenericDocStrings(object):
"""Everything here has a bad docstring
Expand Down Expand Up @@ -502,6 +514,33 @@ def no_punctuation(self):
return "Hello world!"


class BadExamples(object):
Copy link
Member

Choose a reason for hiding this comment

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

I believe there is already an example that imports both of these:

>>> import numpy as np

Any reason we don't leverage that instead of creating a new class?


def numpy_import(self):
"""
Provide example with numpy import.

Examples
--------
This example does not import pandas.
>>> import numpy as np
>>> import datetime
"""
pass

def pandas_import(self):
"""
Provide example with pandas import.

Examples
--------
This example does not import numpy.
>>> import pandas as pd
>>> import pickle
"""
pass


class TestValidator(object):

def _import_path(self, klass=None, func=None):
Expand Down Expand Up @@ -600,7 +639,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),
# Examples tests
('BadExamples', 'numpy_import',
('Examples should not have `import numpy` ',)),
('BadExamples', 'pandas_import',
('Examples should not have `import pandas` ',))
])
def test_bad_examples(self, capsys, klass, func, msgs):
result = validate_one(self._import_path(klass=klass, func=func)) # noqa:F821
Expand Down
11 changes: 11 additions & 0 deletions scripts/validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,12 @@ def examples_errors(self):
error_msgs += f.getvalue()
return error_msgs

@property
def examples_codes(self):
codes = doctest.DocTestParser().get_examples(self.raw_doc)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
codes = doctest.DocTestParser().get_examples(self.raw_doc)
lines = doctest.DocTestParser().get_examples(self.raw_doc)

codes = [line.source for line in codes]
return ' '.join(codes)
Copy link
Member

Choose a reason for hiding this comment

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

Can you return a list with each line instead? We'll reuse this property to validate pep8 too, and returning a single string with all the code joined is not useful in that case.

Also, at least to me examples_codes is somehow misleading. Can you use examples_source_code instead?



def validate_one(func_name):
"""
Expand Down Expand Up @@ -512,6 +518,11 @@ def validate_one(func_name):
examples_errs = doc.examples_errors
if examples_errs:
errs.append('Examples do not pass tests')
examples_codes = doc.examples_codes
if 'import numpy' in examples_codes:
errs.append('Examples should not have `import numpy` ')
if 'import pandas' in examples_codes:
errs.append('Examples should not have `import pandas` ')
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 the error could be more descriptive. pandas does not need to be imported in the examples, as it's assumed to be already imported as pd. I think something like that provides more useful information to devs facing this error.


return {'type': doc.type,
'docstring': doc.clean_doc,
Expand Down