From 17dbd2b830d8bea5d65c69797f6d6ee71eb6cc83 Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Fri, 28 Oct 2016 19:44:52 +0530 Subject: [PATCH 1/7] Add test to check for function parameter description --- sklearn/tests/test_docstring_parameters.py | 112 +++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 sklearn/tests/test_docstring_parameters.py diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py new file mode 100644 index 0000000000000..e4ef6704d9f16 --- /dev/null +++ b/sklearn/tests/test_docstring_parameters.py @@ -0,0 +1,112 @@ +from pkgutil import walk_packages +from importlib import import_module +from numpydoc import docscrape + +import inspect +import warnings + +import sklearn + + +def get_name(func): + parts = [] + module = inspect.getmodule(func) + if module: + parts.append(module.__name__) + if hasattr(func, 'im_class'): + parts.append(func.im_class.__name__) + parts.append(func.__name__) + return '.'.join(parts) + + +def get_all_modules(): + modules = [] + for importer, modname, ispkg in walk_packages(sklearn.__path__, prefix='sklearn.'): + if ispkg: + modules.append(modname) + return modules + + +def check_parameters_match(func, doc=None): + incorrect = [] + name_ = get_name(func) + + # skip deprecated and data descriptors + if not name_.startswith('sklearn.') or 'deprecation' in name_ or inspect.isdatadescriptor(func): + return incorrect + + args = inspect.getargspec(func)[0] + + # drop self + if len(args) > 0 and args[0] == 'self': + args = args[1:] + + if doc is None: + with warnings.catch_warnings(record=True) as w: + doc = docscrape.FunctionDoc(func) + if len(w): + raise RuntimeError('Error for %s:%s' % (name, w[0])) + + param_names = [name for name, _, _ in doc['Parameters']] + param_names = [name.split(':')[0].strip('` ') for name in param_names] + param_names = [name for name in param_names if '*' not in name] + + if len(param_names) != len(args): + mismatch = str(sorted(list(set(args) ^ set(param_names)))) + incorrect += ['For function "' + name_ + '" arg mismatch: ' + mismatch] + else: + args.sort() + param_names.sort() + for n1, n2 in zip(param_names, args): + if n1 != n2: + incorrect += [name_ + ' ' + n1 + ' != ' + n2] + + return incorrect + + +def test_docstring_parameters(): + """Test module docstring formatting""" + public_modules = get_all_modules() + incorrect = [] + + for name in public_modules: + if name.endswith('tests'): + continue + + module = import_module(name, globals()) + + # check for classes in the module + classes = inspect.getmembers(module, inspect.isclass) + for cname, cls in classes: + if cname.startswith('_'): + continue + + with warnings.catch_warnings(record=True) as w: + cdoc = docscrape.ClassDoc(cls) + if len(w): + raise RuntimeError('Error for __init__ of %s in %s:\n%s' + % (cls, name, w[0])) + + # check for __init__ method of class + if hasattr(cls, '__init__'): + incorrect += check_parameters_match(cls.__init__, cdoc) + + # check for all methods of class + for method_name in cdoc.methods: + method = getattr(cls, method_name) + incorrect += check_parameters_match(method) + + # check for _call__ method of class + if hasattr(cls, '__call__'): + incorrect += check_parameters_match(cls.__call__) + + # check for functions in module + functions = inspect.getmembers(module, inspect.isfunction) + for fname, func in functions: + if fname.startswith('_'): + continue + incorrect += check_parameters_match(func) + + msg = '\n' + '\n'.join(sorted(list(set(incorrect)))) + if len(incorrect) > 0: + raise AssertionError(msg) From c29551d1bbadef65933f46960f350b9f230986ac Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Sun, 30 Oct 2016 21:01:01 +0530 Subject: [PATCH 2/7] Add function descriptions --- sklearn/tests/test_docstring_parameters.py | 36 ++++++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index e4ef6704d9f16..faa0a34c91f06 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -9,6 +9,18 @@ def get_name(func): + """ + Returns name of a function + + Parameters + ---------- + func : Python function + + Returns + ------- + name : Function name with complete namespace + + """ parts = [] module = inspect.getmodule(func) if module: @@ -20,6 +32,9 @@ def get_name(func): def get_all_modules(): + """ + Returns all module names of sklearn as an array + """ modules = [] for importer, modname, ispkg in walk_packages(sklearn.__path__, prefix='sklearn.'): if ispkg: @@ -28,6 +43,19 @@ def get_all_modules(): def check_parameters_match(func, doc=None): + """ + Checks docstring for given function + + Parameters + ---------- + func : Function for which docstring is to be checked + doc : (Optional) As returned from numpydoc docscrape + + Returns + ------- + incorrect : Array of error strings + + """ incorrect = [] name_ = get_name(func) @@ -53,7 +81,7 @@ def check_parameters_match(func, doc=None): if len(param_names) != len(args): mismatch = str(sorted(list(set(args) ^ set(param_names)))) - incorrect += ['For function "' + name_ + '" arg mismatch: ' + mismatch] + incorrect += [name_ + ' arg mismatch: ' + mismatch] else: args.sort() param_names.sort() @@ -65,7 +93,9 @@ def check_parameters_match(func, doc=None): def test_docstring_parameters(): - """Test module docstring formatting""" + """ + Test module docstring formatting + """ public_modules = get_all_modules() incorrect = [] @@ -96,7 +126,7 @@ def test_docstring_parameters(): method = getattr(cls, method_name) incorrect += check_parameters_match(method) - # check for _call__ method of class + # check for __call__ method of class if hasattr(cls, '__call__'): incorrect += check_parameters_match(cls.__call__) From e452291586611214e8688681f94db402353c7f90 Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Tue, 1 Nov 2016 10:38:51 +0530 Subject: [PATCH 3/7] List params present in docstring but not in signature separately from the opposite case --- sklearn/tests/test_docstring_parameters.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index faa0a34c91f06..d4f53d67c0fef 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -79,15 +79,19 @@ def check_parameters_match(func, doc=None): param_names = [name.split(':')[0].strip('` ') for name in param_names] param_names = [name for name in param_names if '*' not in name] - if len(param_names) != len(args): - mismatch = str(sorted(list(set(args) ^ set(param_names)))) - incorrect += [name_ + ' arg mismatch: ' + mismatch] - else: - args.sort() - param_names.sort() - for n1, n2 in zip(param_names, args): - if n1 != n2: - incorrect += [name_ + ' ' + n1 + ' != ' + n2] + # parameters that are present in docstring but not in signature + sign_missing_params = sorted(list(set(param_names) - set(args))) + if len(sign_missing_params): + incorrect += [name_ + + ' => params present in docstring but not in signature: ' + + ', '.join(sign_missing_params)] + + # parameters that are present in signature but not in docstring + doc_missing_params = sorted(list(set(args) - set(param_names))) + if len(doc_missing_params): + incorrect += [name_ + + ' => params present in signature but not in docstring: ' + + ', '.join(doc_missing_params)] return incorrect From 8bae9f4984ca0e976d8488930f6b96f52593839d Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Sun, 20 Nov 2016 10:51:55 +0530 Subject: [PATCH 4/7] Move numpydoc to sklearn/externals and ignore externals directory in check --- doc/conf.py | 2 +- .../externals}/numpy_ext/__init__.py | 0 .../externals}/numpy_ext/docscrape.py | 0 .../externals}/numpy_ext/docscrape_sphinx.py | 0 .../externals}/numpy_ext/numpydoc.py | 0 sklearn/externals/setup.py | 1 + sklearn/tests/test_docstring_parameters.py | 9 +++++++-- 7 files changed, 9 insertions(+), 3 deletions(-) rename {doc/sphinxext => sklearn/externals}/numpy_ext/__init__.py (100%) rename {doc/sphinxext => sklearn/externals}/numpy_ext/docscrape.py (100%) rename {doc/sphinxext => sklearn/externals}/numpy_ext/docscrape_sphinx.py (100%) rename {doc/sphinxext => sklearn/externals}/numpy_ext/numpydoc.py (100%) diff --git a/doc/conf.py b/doc/conf.py index e60fc167f9d49..c9a0d1f2b4f66 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -32,7 +32,7 @@ # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. extensions = [ 'sphinx.ext.autodoc', 'sphinx.ext.autosummary', - 'numpy_ext.numpydoc', + 'sklearn.externals.numpy_ext.numpydoc', 'sphinx.ext.linkcode', 'sphinx.ext.doctest', 'sphinx_gallery.gen_gallery', 'sphinx_issues', diff --git a/doc/sphinxext/numpy_ext/__init__.py b/sklearn/externals/numpy_ext/__init__.py similarity index 100% rename from doc/sphinxext/numpy_ext/__init__.py rename to sklearn/externals/numpy_ext/__init__.py diff --git a/doc/sphinxext/numpy_ext/docscrape.py b/sklearn/externals/numpy_ext/docscrape.py similarity index 100% rename from doc/sphinxext/numpy_ext/docscrape.py rename to sklearn/externals/numpy_ext/docscrape.py diff --git a/doc/sphinxext/numpy_ext/docscrape_sphinx.py b/sklearn/externals/numpy_ext/docscrape_sphinx.py similarity index 100% rename from doc/sphinxext/numpy_ext/docscrape_sphinx.py rename to sklearn/externals/numpy_ext/docscrape_sphinx.py diff --git a/doc/sphinxext/numpy_ext/numpydoc.py b/sklearn/externals/numpy_ext/numpydoc.py similarity index 100% rename from doc/sphinxext/numpy_ext/numpydoc.py rename to sklearn/externals/numpy_ext/numpydoc.py diff --git a/sklearn/externals/setup.py b/sklearn/externals/setup.py index 936f0327226d6..f936fc10cb426 100644 --- a/sklearn/externals/setup.py +++ b/sklearn/externals/setup.py @@ -5,5 +5,6 @@ def configuration(parent_package='', top_path=None): from numpy.distutils.misc_util import Configuration config = Configuration('externals', parent_package, top_path) config.add_subpackage('joblib') + config.add_subpackage('numpy_ext') return config diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index d4f53d67c0fef..764f416948347 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -1,6 +1,6 @@ from pkgutil import walk_packages from importlib import import_module -from numpydoc import docscrape +from sklearn.externals.numpy_ext import docscrape import inspect import warnings @@ -8,6 +8,10 @@ import sklearn +_docstring_ignores = [ + 'sklearn.externals' +] + def get_name(func): """ Returns name of a function @@ -60,7 +64,8 @@ def check_parameters_match(func, doc=None): name_ = get_name(func) # skip deprecated and data descriptors - if not name_.startswith('sklearn.') or 'deprecation' in name_ or inspect.isdatadescriptor(func): + if not name_.startswith('sklearn.') or inspect.isdatadescriptor(func) or \ + any(d in name_ for d in _docstring_ignores): return incorrect args = inspect.getargspec(func)[0] From 6733032deebcd7e468da76228f1bd93472dd5276 Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Sun, 20 Nov 2016 11:50:02 +0530 Subject: [PATCH 5/7] Fix pep8 warnings --- sklearn/tests/test_docstring_parameters.py | 37 +++++++++++----------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index 764f416948347..a9fe8514d4329 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -1,5 +1,6 @@ +"""Script to check the discrepancies between docstring parameters and function signature.""" + from pkgutil import walk_packages -from importlib import import_module from sklearn.externals.numpy_ext import docscrape import inspect @@ -12,9 +13,10 @@ 'sklearn.externals' ] + def get_name(func): """ - Returns name of a function + Return name of a function. Parameters ---------- @@ -36,11 +38,10 @@ def get_name(func): def get_all_modules(): - """ - Returns all module names of sklearn as an array - """ + """Return all module names of sklearn as an array.""" modules = [] - for importer, modname, ispkg in walk_packages(sklearn.__path__, prefix='sklearn.'): + for importer, modname, ispkg in \ + walk_packages(sklearn.__path__, prefix='sklearn.'): if ispkg: modules.append(modname) return modules @@ -48,7 +49,7 @@ def get_all_modules(): def check_parameters_match(func, doc=None): """ - Checks docstring for given function + Check docstring for given function. Parameters ---------- @@ -65,7 +66,7 @@ def check_parameters_match(func, doc=None): # skip deprecated and data descriptors if not name_.startswith('sklearn.') or inspect.isdatadescriptor(func) or \ - any(d in name_ for d in _docstring_ignores): + any(d in name_ for d in _docstring_ignores): return incorrect args = inspect.getargspec(func)[0] @@ -78,7 +79,7 @@ def check_parameters_match(func, doc=None): with warnings.catch_warnings(record=True) as w: doc = docscrape.FunctionDoc(func) if len(w): - raise RuntimeError('Error for %s:%s' % (name, w[0])) + raise RuntimeError('Error for %s:%s' % (name_, w[0])) param_names = [name for name, _, _ in doc['Parameters']] param_names = [name.split(':')[0].strip('` ') for name in param_names] @@ -87,24 +88,22 @@ def check_parameters_match(func, doc=None): # parameters that are present in docstring but not in signature sign_missing_params = sorted(list(set(param_names) - set(args))) if len(sign_missing_params): - incorrect += [name_ + - ' => params present in docstring but not in signature: ' + + incorrect += [ + name_ + ' => params present in docstring but not in signature: ' + ', '.join(sign_missing_params)] # parameters that are present in signature but not in docstring doc_missing_params = sorted(list(set(args) - set(param_names))) if len(doc_missing_params): - incorrect += [name_ + - ' => params present in signature but not in docstring: ' + + incorrect += [ + name_ + ' => params present in signature but not in docstring: ' + ', '.join(doc_missing_params)] return incorrect def test_docstring_parameters(): - """ - Test module docstring formatting - """ + """Test module docstring formatting.""" public_modules = get_all_modules() incorrect = [] @@ -112,7 +111,7 @@ def test_docstring_parameters(): if name.endswith('tests'): continue - module = import_module(name, globals()) + module = __import__(name, globals()) # check for classes in the module classes = inspect.getmembers(module, inspect.isclass) @@ -123,8 +122,8 @@ def test_docstring_parameters(): with warnings.catch_warnings(record=True) as w: cdoc = docscrape.ClassDoc(cls) if len(w): - raise RuntimeError('Error for __init__ of %s in %s:\n%s' - % (cls, name, w[0])) + raise RuntimeError( + 'Error for __init__ of %s in %s:\n%s' % (cls, name, w[0])) # check for __init__ method of class if hasattr(cls, '__init__'): From 5e1a488632a8bdc81c140fc98f2052a8fe5476ba Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Sun, 20 Nov 2016 13:37:28 +0530 Subject: [PATCH 6/7] Fix sklearn module import --- sklearn/tests/test_docstring_parameters.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index a9fe8514d4329..860454c6bed87 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -1,4 +1,4 @@ -"""Script to check the discrepancies between docstring parameters and function signature.""" +"""Check discrepancies between docstring parameters and function signature.""" from pkgutil import walk_packages from sklearn.externals.numpy_ext import docscrape @@ -111,8 +111,7 @@ def test_docstring_parameters(): if name.endswith('tests'): continue - module = __import__(name, globals()) - + module = __import__(name, globals(), locals(), ['object'], -1) # check for classes in the module classes = inspect.getmembers(module, inspect.isclass) for cname, cls in classes: From e8346fde9ee3124fd6a3c1c09f62f6b923b38e26 Mon Sep 17 00:00:00 2001 From: Neeraj Gangwar Date: Sun, 20 Nov 2016 14:25:29 +0530 Subject: [PATCH 7/7] Fix value of level in import --- sklearn/tests/test_docstring_parameters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index 860454c6bed87..c29dd5adcdd7c 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -111,7 +111,7 @@ def test_docstring_parameters(): if name.endswith('tests'): continue - module = __import__(name, globals(), locals(), ['object'], -1) + module = __import__(name, globals(), locals(), ['object'], 0) # check for classes in the module classes = inspect.getmembers(module, inspect.isclass) for cname, cls in classes: