DeprecationWarning for inspect.getargspec() raises Error/Fail in pytest-3.6

[dhomeier]

Running the tests for 2.0rc1 I am seeing 24 failures reported, all apparently based on the same

        warnings.warn("inspect.getargspec() is deprecated, "
                      "use inspect.signature() or inspect.getfullargspec()",
>                     DeprecationWarning, stacklevel=2)
E       DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()

../../../../../inspect.py:1041: DeprecationWarning

Setup:

platform darwin -- Python 3.6.1, pytest-3.0.5, py-1.4.32, pluggy-0.4.0

Running tests with Astropy version 2.0rc1.
Running tests in /sw/lib/python3.6/site-packages/astropy.

Date: 2017-06-29T01:55:41

Platform: Darwin-16.6.0-x86_64-i386-64bit

Executable: /sw/bin/python3.6

Full Python Version: 
3.6.1 (default, May  3 2017, 21:41:23) 
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)]

encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15

Numpy: 1.12.1
Scipy: 0.19.1
Matplotlib: 2.0.2
h5py: 2.7.0
Pandas: 0.20.2
Cython: 0.25.2
Using Astropy options: remote_data: none.

rootdir: /sw/lib/python3.6/site-packages/astropy, inifile: 
plugins: hypothesis-3.6.1, backports.unittest-mock-1.2.1

with beautifulsoup 4.6.0.

[pllim]

What is the full traceback? The given info does not point to any Astropy files, so it is hard to track the cause.

[dhomeier]

I could track it down to lxml actually, not beautifulsoup as I had first suspected:

________________________________________________________________ test_read_generic[t/html.html] ________________________________________________________________

filename = 't/html.html'

    @pytest.mark.parametrize('filename', files)
    def test_read_generic(filename):
>       Table.read(os.path.join(ROOT, filename), format='ascii')

/sw/lib/python3.6/site-packages/astropy/io/ascii/tests/test_connect.py:27: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/sw/lib/python3.6/site-packages/astropy/table/table.py:2521: in read
    out = io_registry.read(cls, *args, **kwargs)
/sw/lib/python3.6/site-packages/astropy/io/registry.py:531: in read
    data = reader(*args, **kwargs)
/sw/lib/python3.6/site-packages/astropy/io/ascii/connect.py:22: in read_asciitable
    return read(filename, **kwargs)
/sw/lib/python3.6/site-packages/astropy/io/ascii/ui.py:325: in read
    dat = _guess(table, new_kwargs, format, fast_reader_param)
/sw/lib/python3.6/site-packages/astropy/io/ascii/ui.py:473: in _guess
    dat = reader.read(table)
/sw/lib/python3.6/site-packages/astropy/io/ascii/html.py:345: in read
    return super(HTML, self).read(table)
/sw/lib/python3.6/site-packages/astropy/io/ascii/core.py:1159: in read
    self.lines = self.inputter.get_lines(table)
/sw/lib/python3.6/site-packages/astropy/io/ascii/core.py:310: in get_lines
    return self.process_lines(lines)
/sw/lib/python3.6/site-packages/astropy/io/ascii/html.py:96: in process_lines
    soup = BeautifulSoup('\n'.join(lines))
/sw/lib/python3.6/site-packages/bs4/__init__.py:228: in __init__
    self._feed()
/sw/lib/python3.6/site-packages/bs4/__init__.py:289: in _feed
    self.builder.feed(self.markup)
/sw/lib/python3.6/site-packages/bs4/builder/_lxml.py:250: in feed
    self.parser.feed(markup)
src/lxml/parser.pxi:1217: in lxml.etree._FeedParser.feed (src/lxml/lxml.etree.c:114563)
    ???
src/lxml/parser.pxi:1260: in lxml.etree._FeedParser.feed (src/lxml/lxml.etree.c:113522)
    ???
src/lxml/parser.pxi:850: in lxml.etree._BaseParser._getPushParserContext (src/lxml/lxml.etree.c:109144)
    ???
src/lxml/parser.pxi:867: in lxml.etree._BaseParser._createContext (src/lxml/lxml.etree.c:109352)
    ???
src/lxml/parsertarget.pxi:110: in lxml.etree._TargetParserContext._setTarget (src/lxml/lxml.etree.c:130710)
    ???
src/lxml/parsertarget.pxi:41: in lxml.etree._PythonSaxParserTarget.__cinit__ (src/lxml/lxml.etree.c:129176)
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

func = <bound method LXMLTreeBuilderForXML.start of <bs4.builder._lxml.LXMLTreeBuilder object at 0x11430ed30>>

    def getargspec(func):
        """Get the names and default values of a function's parameters.
    
        A tuple of four things is returned: (args, varargs, keywords, defaults).
        'args' is a list of the argument names, including keyword-only argument names.
        'varargs' and 'keywords' are the names of the * and ** parameters or None.
        'defaults' is an n-tuple of the default values of the last n parameters.
    
        This function is deprecated, as it does not support annotations or
        keyword-only parameters and will raise ValueError if either is present
        on the supplied callable.
    
        For a more structured introspection API, use inspect.signature() instead.
    
        Alternatively, use getfullargspec() for an API with a similar namedtuple
        based interface, but full support for annotations and keyword-only
        parameters.
        """
        warnings.warn("inspect.getargspec() is deprecated, "
                      "use inspect.signature() or inspect.getfullargspec()",
>                     DeprecationWarning, stacklevel=2)
E       DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()

/sw/lib/python3.6/inspect.py:1041: DeprecationWarning

The real culprit is this function in lxml-3.8.0's parsertarget.pxi:

cdef object inspect_getargspec
try:
    from inspect import getargspec as inspect_getargspec
except ImportError:
    from inspect import getfullargspec as inspect_getargspec

This might be their problem upstream then, as they should perhaps try getfullargspec (which exists at least as of 3.4) first, but I don't quite understand why the DeprecationWarning (which already is present in Python3.5's inspect) raises an error in the Python3.6 test suite - anything one might change in the test setup to avoid this?

[pllim]

One thing we can do on our side is to mark that test as xfail for Python 3.6 until this is fixed upstream. Strictly speaking, one can also check for lxml version for the xfail condition but I don't feel comfortable introducing that dependency in the test because lxml is not an official Astropy dependency.

@taldcroft , @bsipocz , or whoever interested to chime in -- What do you think?

[bsipocz]

xfailing that one test for python3.6 sounds good to me. Please leave a descriptive note about the lxml version issue so we will have a chance to revisit when needed (and will also see why it xpasses the xfail...)

[dhomeier]

Thought of xfail as a kind of last resort, too - since the code has not changed so far in the lxml git repo, it would be difficult to predict in which version it will be fixed.
Instead of xfail I think one might also insert a with pytest.warns(DeprecationWarning) for 3.5 and 3.6, but then this would break again whenever lxml is changed...

[pllim]

Actually, instead of xfail-ing the whole test, you can first catch the warning and if it is DeprecationWarning and Python is 3.6, then xfail it -- This perhaps will reduce the number of xpass for people who have "good" lxml version.

[dhomeier]

Unfortunately it seems difficult to catch the warning in the test.
A bit confusingly to me, pytest's

E       DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()

made it look as it is promoted to an Error somewhere along the way down, but that just seems to be the reporting format. Anyway the decorator approach just fails to raise the warning:

                with pytest.warns(DeprecationWarning):
>                   table = ascii.read(testfile['name'], guess=True, **guess_opts)
E                   Failed: DID NOT WARN

test_read.py:180: Failed

[dhomeier]

Hmm, looks like it has something to do with the warnings.filters setting, because confusingly I saw neither an exception nor a warning when trying

from inspect import getargspec as inspect_getargspec

directly from the python3.6 interpreter. These are the filter settings when starting a new Python 3.6 sessions:

>>> import warnings
>>> warnings.filters
[('ignore', None, <class 'DeprecationWarning'>, None, 0), ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0), ('ignore', None, <class 'ImportWarning'>, None, 0), ('ignore', None, <class 'BytesWarning'>, None, 0), ('ignore', None, <class 'ResourceWarning'>, None, 0)]
>>> import astropy
>>> warnings.filters
[('ignore', re.compile('numpy.ndarray size changed', re.IGNORECASE), <class 'Warning'>, re.compile(''), 0), ('ignore', re.compile('numpy.ufunc size changed', re.IGNORECASE), <class 'Warning'>, re.compile(''), 0), ('ignore', re.compile('numpy.dtype size changed', re.IGNORECASE), <class 'Warning'>, re.compile(''), 0), ('always', None, <class 'numpy.lib.polynomial.RankWarning'>, None, 0), ('ignore', None, <class 'DeprecationWarning'>, None, 0), ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0), ('ignore', None, <class 'ImportWarning'>, None, 0), ('ignore', None, <class 'BytesWarning'>, None, 0), ('ignore', None, <class 'ResourceWarning'>, None, 0), ('ignore', re.compile('', re.IGNORECASE), <class 'pkg_resources.PEP440Warning'>, re.compile(''), 0)]

and after running the io.ascii tests (with the failures):

>>> warnings.filters
[('ignore', re.compile('inspect\\.getargspec\\(\\) is deprecated, use inspect\\.signature\\(\\) instead', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile(''), 0), ('ignore', re.compile("'U' mode is deprecated", re.IGNORECASE), <class 'DeprecationWarning'>, re.compile(''), 0), ('ignore', re.compile('The value of convert_charrefs will become True in 3\\.5\\. You are encouraged to set the value explicitly\\.', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile(''), 0), ('ignore', re.compile('The strict argument and mode are deprecated\\.', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile(''), 0), ('error', re.compile('.*', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile(''), 0)]

Exactly the same in 3.5, except the tests do not fail...

Ah, I think now I see why - at some point in the tests the general DeprecationWarning filter is replaced by four filters for specific warnings, and all other are turned into
('error', re.compile('.*', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile(''), 0)
But since the inspect warning between 3.5 and 3.6 was changed from "use inspect.signature() instead" to "use inspect.signature() or inspect.getfullargspec()", the filter fails in the latter.

Any ideas if this is part of the astropy-specific pytest setup, or where to change it?

[pllim]

Ohhhh! Yes, it is at https://github.com/astropy/astropy/blob/master/astropy/tests/helper.py#L139 . Since there is a precedent, there is no need to xfail but simply add to that part of the code as follows:

_warnings_to_ignore_by_pyver = {
    # ...
    (3, 6): set([
        # Some note on why this was added
        r"put the text here; see other entries for example"])
}

[bsipocz]

Brilliant detective work! Indeed it works now.
Can you @dhomeier open a PR, or shall do it?

[dhomeier]

Just found it, too - yes, just requires a copy of @mdboom's fix for the 3.5 version.

Whatever is easier - this is my patch:

diff --git a/astropy/tests/helper.py b/astropy/tests/helper.py
index eeee8e9dc..a87f68fd7 100644
--- a/astropy/tests/helper.py
+++ b/astropy/tests/helper.py
@@ -150,7 +150,13 @@ _warnings_to_ignore_by_pyver = {
         # This can be removed when fixed in py.test.
         # See https://github.com/pytest-dev/pytest/pull/1009
         r"inspect\.getargspec\(\) is deprecated, use "
-        r"inspect\.signature\(\) instead"])}
+        r"inspect\.signature\(\) instead"]),
+    (3, 6): set([
+        # py.test raises this slightly different warning on Python 3.6.
+        # This can be removed when fixed in py.test.
+        # See also https://github.com/pytest-dev/pytest/pull/1009
+        r"inspect\.getargspec\(\) is deprecated, use "
+        r"inspect\.signature\(\) or inspect\.getfullargspec\(\)"])}
 
 

[bsipocz]

It's your fix, so please push it, and we can merge when CI passed.

[dhomeier]

Hope branching this from v2.0rc1 was the right thing now...

[bsipocz]

Oh, good that you say that, I haven't noticed. The base for every PR should be master, we'll do the backporting separately.
However I can change the base on your PR.

[bsipocz]

PR is fixed up and can be merged once CI passed.