Repository: sympy/sympy
Pull request: Coverage doctest fix
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
Fixes #16716
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
Coverage_doctest.py was failing due to name changes between python2 and python3; this pull request updates the names to the python3 versions. It also was not dealing with dynamic docstrings correctly; now it skips them. Variables also have been updated to be more descriptive. 


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->


Problem statement:
bin/coverage_doctest.py does not work
```pytb
$./bin/coverage_doctest.py
DOCTEST COVERAGE for /Users/aaronmeurer/Documents/Python/sympy/sympy/sympy
======================================================================

Traceback (most recent call last):
  File "./bin/coverage_doctest.py", line 601, in <module>
    no_color=args.no_color, sphinx=args.sphinx)
  File "./bin/coverage_doctest.py", line 541, in go
    verbose=verbose, no_color=no_color, exact=False, sphinx=sphinx)
  File "./bin/coverage_doctest.py", line 549, in go
    any(name in file for name in skip_paths))):
NameError: name 'skip_paths' is not defined
```

It works if you remove the skip_paths check, but we should look into the history of the file to see what that was, because clearly something broke at some point.

Hints from issue discussion:
> look into the history of the file to see what that was

It was initially a way to skip testing of mpmath in 24eeabe2fe23a1d5103e61c3b8d1ed10c44d9795; it was removed in 9b204c3d1a3e4f6753a7d5186f3074ef13641f36 when mpmath (the only thing being skipped) was removed. The simple fix is to just define `skip_paths` as an empty list again.
https://github.com/sympy/sympy/pull/16729#issuecomment-487160414
I'm not sure that every not frequently used utilities are working.
For example `diagnose_imports` is failing
```
python bin/diagnose_imports
Traceback (most recent call last):

    from sympy.core.compatibility import builtins
ModuleNotFoundError: No module named 'sympy'
```
And also, I see some utilities have optional arguments, but are they correctly tested?
I think that every executable utilites may need to be tested, if there can be a way to do.
Hi, I would like to work on this issue if there isn't any existing pull request for the same. Can someone guide me a little bit regarding this?
I don't think there are any existing pull requests. You should start by running the script and posting whatever output you get here. Then we can figure out what needs to be done to fix whatever errors come up. 
@asmeurer this is what I am getting. From what I understand of this, the object returned by ```inspect.getfullargspec(fobj)``` does not have a keywords attribute. 
```
(sympy-dev) D:\Documents\sympy>python bin/coverage_doctest.py --no-sphinx
DOCTEST COVERAGE for D:\Documents\sympy\sympy
======================================================================

sympy.abc: ←[0;33mDoctests:←[0m ←[0;32m100% (0 of 0)←[0m
Traceback (most recent call last):
  File "bin/coverage_doctest.py", line 623, in <module>
    doctests, total_sphinx, num_functions = go(sympy_top, file, verbose=args.verbose,
  File "bin/coverage_doctest.py", line 562, in go
    _doctests, _total_sphinx,  _num_functions = go(sympy_top, '%s/%s' % (file, F),
  File "bin/coverage_doctest.py", line 562, in go
    _doctests, _total_sphinx,  _num_functions = go(sympy_top, '%s/%s' % (file, F),
  File "bin/coverage_doctest.py", line 579, in go
    return coverage(get_mod_name(file, sympy_top), verbose=verbose,
  File "bin/coverage_doctest.py", line 509, in coverage
    f_dt, f = process_function(f_name, member, obj,
  File "bin/coverage_doctest.py", line 311, in process_function
    full_name = _get_arg_list(name, obj)
  File "bin/coverage_doctest.py", line 231, in _get_arg_list
    if argspec.keywords:
AttributeError: 'FullArgSpec' object has no attribute 'keywords'
```
Hi, I've noticed that no one seems to be working on this issue right now. Is it okay if I try to work on it?
Yes you can. 
I think I may have figured out how to fix it already! It looks to me like the issue is that on line 231, `if argspec.keywords:
` is failing because sometimes `keywords` isn't in `argspec`, and changing it to ` if hasattr(argspec, 'keywords') and argspec.keywords:` should fix it.
You should rerun the script to check what the actual issues are. Some of the errors listed here may no longer be accurate. Any problems are likely due to the script being written for an older version of Python. It only needs to support the newest version of Python.

Commit message: Removed commented out code

Patch to review:
diff --git a/bin/coverage_doctest.py b/bin/coverage_doctest.py
index 87e4eb74cefb..0e7399414a8b 100755
--- a/bin/coverage_doctest.py
+++ b/bin/coverage_doctest.py
@@ -72,8 +72,8 @@ def print_header(name, underline=None, color=None):
         print(underline*len(name))
 
 
-def print_coverage(module_path, c, c_md, c_mdt, c_idt, c_sph, f, f_md, f_mdt,
-                   f_idt, f_sph, score, total_doctests, total_members,
+def print_coverage(module_path, c, c_missing_doc, c_missing_doctest, c_indierect_doctest, c_sph, f, f_missing_doc, f_missing_doctest,
+                   f_indierect_doctest, f_sph, score, total_doctests, total_members,
                    sphinx_score, total_sphinx, verbose=False, no_color=False,
                    sphinx=True):
     """ Prints details (depending on verbose) of a module """
@@ -137,19 +137,19 @@ def print_coverage(module_path, c, c_md, c_mdt, c_idt, c_sph, f, f_md, f_mdt,
             print_header('No classes found!')
 
         else:
-            if c_md:
+            if c_missing_doc:
                 print_header('Missing docstrings', '-', not no_color and small_header_color)
-                for md in c_md:
+                for md in c_missing_doc:
                     print('  * ' + md)
-            if c_mdt:
+            if c_missing_doctest:
                 print_header('Missing doctests', '-', not no_color and small_header_color)
-                for md in c_mdt:
+                for md in c_missing_doctest:
                     print('  * ' + md)
-            if c_idt:
+            if c_indierect_doctest:
                 # Use "# indirect doctest" in the docstring to
                 # suppress this warning.
                 print_header('Indirect doctests', '-', not no_color and small_header_color)
-                for md in c_idt:
+                for md in c_indierect_doctest:
                     print('  * ' + md)
                 print('\n    Use \"# indirect doctest\" in the docstring to suppress this warning')
             if c_sph:
@@ -161,17 +161,17 @@ def print_coverage(module_path, c, c_md, c_mdt, c_idt, c_sph, f, f_md, f_mdt,
         if not f:
             print_header('No functions found!')
         else:
-            if f_md:
+            if f_missing_doc:
                 print_header('Missing docstrings', '-', not no_color and small_header_color)
-                for md in f_md:
+                for md in f_missing_doc:
                     print('  * ' + md)
-            if f_mdt:
+            if f_missing_doctest:
                 print_header('Missing doctests', '-', not no_color and small_header_color)
-                for md in f_mdt:
+                for md in f_missing_doctest:
                     print('  * ' + md)
-            if f_idt:
+            if f_indierect_doctest:
                 print_header('Indirect doctests', '-', not no_color and small_header_color)
-                for md in f_idt:
+                for md in f_indierect_doctest:
                     print('  * ' + md)
                 print('\n    Use \"# indirect doctest\" in the docstring to suppress this warning')
             if f_sph:
@@ -228,8 +228,8 @@ def _get_arg_list(name, fobj):
     # Add var args
     if argspec.varargs:
         arg_list.append(argspec.varargs)
-    if argspec.keywords:
-        arg_list.append(argspec.keywords)
+    if argspec.varkw:
+        arg_list.append(argspec.varkw)
 
     # Truncate long arguments
     arg_list = [x[:trunc] for x in arg_list]
@@ -283,20 +283,20 @@ def find_sphinx(name, mod_path, found={}):
     found[mod_path] = p.is_imported
     return name in p.is_imported
 
-def process_function(name, c_name, b_obj, mod_path, f_sk, f_md, f_mdt, f_idt,
-                     f_has_doctest, sk_list, sph, sphinx=True):
+def process_function(name, c_name, b_obj, mod_path, f_skip, f_missing_doc, f_missing_doctest, f_indierect_doctest,
+                     f_has_doctest, skip_list, sph, sphinx=True):
     """
     Processes a function to get information regarding documentation.
     It is assume that the function calling this subrouting has already
     verified that it is a valid module function.
     """
-    if name in sk_list:
+    if name in skip_list:
         return False, False
 
     # We add in the end, as inspect.getsourcelines is slow
-    add_md = False
-    add_mdt = False
-    add_idt = False
+    add_missing_doc = False
+    add_missing_doctest = False
+    add_indierect_doctest = False
     in_sphinx = True
     f_doctest = False
     function = False
@@ -311,30 +311,30 @@ def process_function(name, c_name, b_obj, mod_path, f_sk, f_md, f_mdt, f_idt,
     full_name = _get_arg_list(name, obj)
 
     if name.startswith('_'):
-        f_sk.append(full_name)
+        f_skip.append(full_name)
     else:
         doc = obj.__doc__
-        if type(doc) is str:
+        if isinstance(doc, str):
             if not doc:
-                add_md = True
+                add_missing_doc = True
             elif not '>>>' in doc:
-                add_mdt = True
+                add_missing_doctest = True
             elif _is_indirect(name, doc):
-                add_idt = True
+                add_indierect_doctest = True
             else:
                 f_doctest = True
         elif doc is None:
             # this was a function defined in the docstring
             f_doctest = True
         else:
-            assert None, type(doc)
+            raise TypeError('Current doc type for ', print(obj), ' is ', type(doc), '. Docstring must be a string, property, or none')
 
         function = True
 
         if sphinx:
             in_sphinx = find_sphinx(obj_name, mod_path)
 
-    if add_md or add_mdt or add_idt or not in_sphinx:
+    if add_missing_doc or add_missing_doctest or add_indierect_doctest or not in_sphinx:
         try:
             line_no = inspect.getsourcelines(obj)[1]
         except IOError:
@@ -343,19 +343,19 @@ def process_function(name, c_name, b_obj, mod_path, f_sk, f_md, f_mdt, f_idt,
             return False, False
 
         full_name = "LINE %d: %s" % (line_no, full_name)
-        if add_md:
-            f_md.append(full_name)
-        elif add_mdt:
-            f_mdt.append(full_name)
-        elif add_idt:
-            f_idt.append(full_name)
+        if add_missing_doc:
+            f_missing_doc.append(full_name)
+        elif add_missing_doctest:
+            f_missing_doctest.append(full_name)
+        elif add_indierect_doctest:
+            f_indierect_doctest.append(full_name)
         if not in_sphinx:
             sph.append(full_name)
 
     return f_doctest, function
 
 
-def process_class(c_name, obj, c_sk, c_md, c_mdt, c_idt, c_has_doctest,
+def process_class(c_name, obj, c_skip, c_missing_doc, c_missing_doctest, c_indierect_doctest, c_has_doctest,
                   mod_path, sph, sphinx=True):
     """
     Extracts information about the class regarding documentation.
@@ -365,7 +365,7 @@ def process_class(c_name, obj, c_sk, c_md, c_mdt, c_idt, c_has_doctest,
 
     # Skip class case
     if c_name.startswith('_'):
-        c_sk.append(c_name)
+        c_skip.append(c_name)
         return False, False, None
 
     c = False
@@ -381,13 +381,13 @@ def process_class(c_name, obj, c_sk, c_md, c_mdt, c_idt, c_has_doctest,
     c = True
     full_name = "LINE %d: %s" % (line_no, c_name)
     doc = obj.__doc__
-    if type(doc) is str:
+    if isinstance(doc, str):
         if not doc:
-            c_md.append(full_name)
+            c_missing_doc.append(full_name)
         elif not '>>>' in doc:
-            c_mdt.append(full_name)
+            c_missing_doctest.append(full_name)
         elif _is_indirect(c_name, doc):
-            c_idt.append(full_name)
+            c_indierect_doctest.append(full_name)
         else:
             c_dt = True
             c_has_doctest.append(full_name)
@@ -395,8 +395,12 @@ def process_class(c_name, obj, c_sk, c_md, c_mdt, c_idt, c_has_doctest,
         # this was a class defined in the docstring
         c_dt = True
         c_has_doctest.append(full_name)
+    elif isinstance(doc, property):
+        # skip class with dynamic doc
+        c_skip.append(c_name)
+        return False, False, None
     else:
-        assert None, type(doc)
+        raise TypeError('Current doc type of ', print(obj), ' is ', type(doc), '. Docstring must be a string, property , or none')
 
     in_sphinx = False
     if sphinx:
@@ -425,19 +429,19 @@ def coverage(module_path, verbose=False, no_color=False, sphinx=True):
         return 0, 0, 0
 
     c_skipped = []
-    c_md = []
-    c_mdt = []
+    c_missing_doc = []
+    c_missing_doctest = []
     c_has_doctest = []
-    c_idt = []
+    c_indierect_doctest = []
     classes = 0
     c_doctests = 0
     c_sph = []
 
     f_skipped = []
-    f_md = []
-    f_mdt = []
+    f_missing_doc = []
+    f_missing_doctest = []
     f_has_doctest = []
-    f_idt = []
+    f_indierect_doctest = []
     functions = 0
     f_doctests = 0
     f_sph = []
@@ -465,7 +469,7 @@ def coverage(module_path, verbose=False, no_color=False, sphinx=True):
         if inspect.isfunction(obj) or inspect.ismethod(obj):
 
             f_dt, f = process_function(member, '', obj, module_path,
-                f_skipped, f_md, f_mdt, f_idt, f_has_doctest, skip_members,
+                f_skipped, f_missing_doc, f_missing_doctest, f_indierect_doctest, f_has_doctest, skip_members,
                 f_sph, sphinx=sphinx)
             if f:
                 functions += 1
@@ -476,8 +480,8 @@ def coverage(module_path, verbose=False, no_color=False, sphinx=True):
         elif inspect.isclass(obj):
 
             # Process the class first
-            c_dt, c, source = process_class(member, obj, c_skipped, c_md,
-                c_mdt, c_idt, c_has_doctest, module_path, c_sph, sphinx=sphinx)
+            c_dt, c, source = process_class(member, obj, c_skipped, c_missing_doc,
+                c_missing_doctest, c_indierect_doctest, c_has_doctest, module_path, c_sph, sphinx=sphinx)
             if not c:
                 continue
             else:
@@ -507,7 +511,7 @@ def coverage(module_path, verbose=False, no_color=False, sphinx=True):
                 if inspect.isfunction(f_obj) or inspect.ismethod(f_obj):
 
                     f_dt, f = process_function(f_name, member, obj,
-                        module_path, f_skipped, f_md, f_mdt, f_idt, f_has_doctest,
+                        module_path, f_skipped, f_missing_doc, f_missing_doctest, f_indierect_doctest, f_has_doctest,
                         skip_members, f_sph, sphinx=sphinx)
                     if f:
                         functions += 1
@@ -535,16 +539,16 @@ def coverage(module_path, verbose=False, no_color=False, sphinx=True):
         sphinx_score = 0
 
     # Sort functions/classes by line number
-    c_md = sorted(c_md, key=lambda x: int(x.split()[1][:-1]))
-    c_mdt = sorted(c_mdt, key=lambda x: int(x.split()[1][:-1]))
-    c_idt = sorted(c_idt, key=lambda x: int(x.split()[1][:-1]))
+    c_missing_doc = sorted(c_missing_doc, key=lambda x: int(x.split()[1][:-1]))
+    c_missing_doctest = sorted(c_missing_doctest, key=lambda x: int(x.split()[1][:-1]))
+    c_indierect_doctest = sorted(c_indierect_doctest, key=lambda x: int(x.split()[1][:-1]))
 
-    f_md = sorted(f_md, key=lambda x: int(x.split()[1][:-1]))
-    f_mdt = sorted(f_mdt, key=lambda x: int(x.split()[1][:-1]))
-    f_idt = sorted(f_idt, key=lambda x: int(x.split()[1][:-1]))
+    f_missing_doc = sorted(f_missing_doc, key=lambda x: int(x.split()[1][:-1]))
+    f_missing_doctest = sorted(f_missing_doctest, key=lambda x: int(x.split()[1][:-1]))
+    f_indierect_doctest = sorted(f_indierect_doctest, key=lambda x: int(x.split()[1][:-1]))
 
-    print_coverage(module_path, classes, c_md, c_mdt, c_idt, c_sph, functions, f_md,
-                   f_mdt, f_idt, f_sph, score, total_doctests, total_members,
+    print_coverage(module_path, classes, c_missing_doc, c_missing_doctest, c_indierect_doctest, c_sph, functions, f_missing_doc,
+                   f_missing_doctest, f_indierect_doctest, f_sph, score, total_doctests, total_members,
                    sphinx_score, total_sphinx, verbose=verbose,
                    no_color=no_color, sphinx=sphinx)
 


Code review comments:
diff_hunk: @@ -425,19 +429,19 @@ def coverage(module_path, verbose=False, no_color=False, sphinx=True):
         return 0, 0, 0
 
     c_skipped = []
-    c_md = []
-    c_mdt = []
+    c_missing_doc = []
+    c_missing_doctest = []
     c_has_doctest = []
-    c_idt = []
+    c_indierect_doctest = []
     classes = 0
     c_doctests = 0
     c_sph = []
 
     f_skipped = []
-    f_md = []
-    f_mdt = []
+    f_missing_doc = []
+    f_missing_doctest = []
     f_has_doctest = []
-    f_idt = []
+    f_indierect_doctest = []
path: bin/coverage_doctest.py
text: @user1:
"indirect" is spelled wrong in these variable names.