The passenv and setenv in the [tox] section at the top of tox.ini aren't doing what's intended (they aren't doing anything, I think).
The intention (as stated by @Zac-HD in https://github.com/HypothesisWorks/hypothesis-python/pull/1122#issuecomment-366567315) is:
... the
PYTHONWARNINGSbit should use the user's environment if the variable is set; but make those warnings into errors if it isn't.
Based on http://tox.readthedocs.io/en/latest/example/pytest.html and my experiments, it's easy to achieve that intention: Just move this stuff to the [testenv] section a few lines below.
However, if we do that currently, then a user running tox with PYTHONWARNINGS unset won't even get as far as running any tests due to the deprecation warning:
$make check-fast
ln -sf /Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/bin/flake8 /Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/flake8
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/flake8 src tests
rm -f /Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/tox
ln -sf /Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/bin/tox /Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/tox
touch /Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/bin/tox /Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/tox
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/tox --recreate -e pypy-brief
GLOB sdist-make: /Users/davidchudzicki/hypothesis-python/setup.py
pypy-brief recreate: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/pypy-brief
ERROR: invocation failed (exit code 1), logfile: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/pypy-brief/log/pypy-brief-0.log
ERROR: actionid: pypy-brief
msg: getenv
cmdargs: ['/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/bin/python3.6', '-m', 'virtualenv', '--python', '/Users/davidchudzicki/.cache/hypothesis-build-runtimes/snakepit/pypy', 'pypy-brief']
Failed to import the site module
Traceback (most recent call last):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/lib/python3.6/site.py", line 703, in <module>
main()
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/lib/python3.6/site.py", line 683, in main
paths_in_sys = addsitepackages(paths_in_sys)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/lib/python3.6/site.py", line 282, in addsitepackages
addsitedir(sitedir, known_paths)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/lib/python3.6/site.py", line 204, in addsitedir
addpackage(sitedir, name, known_paths)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/lib/python3.6/site.py", line 165, in addpackage
f = open(fullname, "rU")
DeprecationWarning: 'U' mode is deprecated
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/bin/python3.6 -m virtualenv --python /Users/davidchudzicki/.cache/hypothesis-build-runtimes/snakepit/pypy pypy-brief (see /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/pypy-brief/log/pypy-brief-0.log)', 1)
_________________________________________________________________________________________________ summary __________________________________________________________________________________________________
ERROR: pypy-brief: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('/Users/davidchudzicki/.cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/bin/python3.6 -m virtualenv --python /Users/davidchudzicki/.cache/hypothesis-build-runtimes/snakepit/pypy pypy-brief (see /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/pypy-brief/log/pypy-brief-0.log)', 1)
make: *** [check-fast] Error 1
Wow am I reading that right that that's a use of a deprecated API from within the python standard library itself? 😞
Hmm, apparently that line was changed 7 years ago and I still have the old one when I run tox.
I'm not sure why I have the old version in .cache/hypothesis-build-runtimes/virtualenvs/tools-6d7a8f3c9c/lib/python3.6/site.py, or whether that's something weird that's unique to me. Sorry if I'm the one causing confusion here.
Everywhere I have the new version.
Looks like this is because virtualenv has an old embedded version of site.py:
https://github.com/pypa/virtualenv/issues/555 & https://github.com/tox-dev/tox/issues/436.
In case anyone's interested in an update on this:
I don't know a good fix for the site.py issue so I'm thinking of seeing if we can at least make FutureWarning an error. The current issues with that are:
import pandas fails (https://github.com/pandas-dev/pandas/issues/19944)test_timeout_traceback_is_hidden in pytest/ is a bit of an issue since it tests the output of a test that necessarily uses @settings(timeout=1) (which is deprecated) -- surely working around that is doable but seems a little awkward.Both of these are coming from basic-test.sh. One option is to surround both the pytest bit and the pandas bit with something like this:
export OLD_PYTHONWARNINGS=$PYTHONWARNINGS
export PYTHONWARNINGS="default"
$PYTEST --runpytest=subprocess tests/pytest
export PYTHONWARNINGS=$OLD_PYTHONWARNINGS
I'll respond here instead of on the pull, just to keep the conversation in one place. (just mention if you'd rather move it)
It turns out that there is a fix for the site.py problem! tox-dev/tox#630 culminates in the publication of the tox-venv package, which drops support for Pythons 2.6 and 3.3 (who cares!) and fixes our issue here. Try it out, and if it works we should also leave feedback on that issue.
For the pandas warning, would PYTHONWARNINGS="default" $PYTEST --runpytest=subprocess tests/pandas work? That is, setting it for one command instead of exporting the var.
For test_timeout_traceback_is_hidden, I'd wrap the whole thing into another function and decorate that with @checks_deprecated_behaviour. This is indeed awkward, but should then be properly environment-independent.
I guess I failed to read to the end the first time I saw that thread (thanks @Zac-HD for noticing it has a happy ending! I'm sure lots of other people will appreciate your update in https://github.com/tox-dev/tox/issues/436#issuecomment-369573251 too). I'll give it a try in the next few days.
would
PYTHONWARNINGS="default" $PYTEST --runpytest=subprocess tests/pandaswork?
Yes, sorry, that works for both is is actually what's in the PR. I got it wrong in the comment here but noticed the improvement before pushing.
I'd wrap the whole thing into another function and decorate that with
@checks_deprecated_behaviour.
My initial attempt that when I was first trying to get it working didn't pan out. But I'll try again.
Hey Zac,
I got test_timeout_traceback_is_hidden working without messing with the environment: You can see in the PR, but:
@checks_deprecated_behaviour doesn't look easy to import from there (because the test is run from a random tempdir) so I used warnings.catch_warnings directly insteadwarnings.catch_warnings to surround both the test definition and the test execution -- a yield fixture seemed to be the way to achieve the latter, although I recognize it's getting pretty complicated-looking now. ☹️ I also got pandas working without an extra environment change by saying in the top-level setenv that we want to ignore FutureWarnings from pandas._version. I think that's better than what I had before.
I did make an attempt at using tox-venv to bypass virtualenv's DeprecationWarning. After various tracebacks from other stuff, I think this was enough to get make check-py36 working:
PYTHONWARNINGS={env:PYTHONWARNINGS:error::DeprecationWarning,default::DeprecationWarning:pip.pep425tags,default::DeprecationWarning:pip.wheel,default::DeprecationWarning:setuptools.depends,default::DeprecationWarning:setuptools.command.egg_info,default::DeprecationWarning:_pytest.assertion.rewrite,default::DeprecationWarning:_pytest.fixtures::FutureWarning}
(I found and commented on the existing issue for one of those, but honestly didn't feel like digging into the rest.)
tox-venv itself worked great for Python 3.6 (all we need to do is add it to the requirements/tools.txt), but I didn't quite understand whether I could use it for our Python 2 builds (which still use virtualenv even with this).
And honestly, I'm becoming a little skeptical about whether keeping track of everyone's DeprecationWarnings makes sense for us. I'm especially loathe to create extra work for other people (you or David or whoever) who will end up dealing with more warnings that we have little control over down the line. I'm even a little worried about that with https://github.com/HypothesisWorks/hypothesis-python/pull/1149 (and won't be bothered by a suggestion that we shouldn't merge it), but it seems to be a much bigger deal with DeprecationWarning than FutureWarning.
Obligatory: I'm absolutely not criticizing the people, mostly volunteers, maintaining the libraries that call stuff with deprecation warnings. Just noting that the ecosystem is such that this could lead to a fair amount of low-benefit work for us right now.
I'm also not trying to deny that other packages' deprecation warnings could be an issue for our users. An upstream dependeny of ours that calls deprecated stuff can affect our users just as much as us calling deprecated stuff. I just think we are (or at least I am) much less likely to do something about those.
(I'm pretty open to helping solve it if you'd like us to, but this is where I've landed for now...)
It'd be pretty cool if there were a way to only get deprecation warnings coming directly from the package we're calling, and not ones that are just passed through the package we call. I might look around for something like that later, but haven't yet. 😄
:tada: to the pytest and pandas fixes - they look good to me.
tox-venv just falls back to the standard system when you use it on Python 2, so we should be fine just using it unconditionally. Literally the only downsides are (a) you have to install it, and (b) it only supports non-EOL versions of Python - neither of which are problems for us.
And honestly, I'm becoming a little sceptical about whether keeping track of everyone's
DeprecationWarnings makes sense for us.
Fair enough! Part of my motivation here is that if we run CI with warnings as errors, it becomes easier for everyone else downstream - whether that's because we fixed or got something fixed, or just did the research.
I suspect that most of these deprecation warnings are because we're using old versions of pip, setuptools, and wheel. Proposed action:
Cool, thanks for pushing me to do the DeprecationWarning bit too. 😄
I really like your idea for the post, and would be excited to do it. A couple thoughts:
On the actual PR (and as notes that might be interesting to your, or useful to refer back to when we write the post):
We're already using the latest pip (9.0.1) and wheel (0.30). One error would indeed be fixed by upgrading setuptools, but I chose to ignore it instead: I think (?) our setuptools version is coming from the default py36 tox environment, and if we want to change that I think we should do it in a separate PR.
Note that (I think) none of the external errors are warnings our users will get from using hypothesis - they're just stuff in our build/test pipeline. All of these came from running make check-py36 without the relevant ignore. Here's some more detail about the various errors (I've also put links to issues in the comments of tox.ini):
(python27 only) error on import pandas (latest, 0.22) (filed issue)
Issue: https://github.com/pandas-dev/pandas/issues/19944
Ignored via default::FutureWarning:pandas._version.
They added this to the 0.23 milestone, and there's also a comment that it would be irrelevant as of 0.24.
import pip (latest: 9.0.1) uses deprecated imp module (commented on relevant issue)
Issue: https://github.com/pypa/pip/issues/5013
Ignored via default::DeprecationWarning:pip.pep425tags.
It might be good to file a new issue so this isn't burried under a title that sounds like it's only about Python 3.7.
pip (latest: 9.0.1) using deprecated
readfp (filed an issue)
Issue: https://github.com/pypa/pip/issues/5056
I'm pretty sure pip doesn't have an issue for this yet, but I had trouble constructing a minimal example so I haven't filed an issue.
Ignored via default::DeprecationWarning:pip.wheel:
Traceback (most recent call last):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/basecommand.py", line 215, in main
status = self.run(options, args)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/commands/install.py", line 342, in run
prefix=options.prefix_path,
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/req/req_set.py", line 784, in install
**kwargs
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/req/req_install.py", line 851, in install
self.move_wheel_files(self.source_dir, root=root, prefix=prefix)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/req/req_install.py", line 1064, in move_wheel_files
isolated=self.isolated,
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/wheel.py", line 351, in move_wheel_files
console, gui = get_entrypoints(ep_file)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/wheel.py", line 229, in get_entrypoints
cp.readfp(data)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/configparser.py", line 761, in readfp
DeprecationWarning, stacklevel=2
DeprecationWarning: This method will be removed in future versions. Use 'parser.read_file()' instead.
setuptools (latest, 38.5.1) uses deprecated imp module (there's already an issue)
Issue: https://github.com/pypa/setuptools/issues/479
Ignored via default::DeprecationWarning:setuptools.depends.
We're on on 28.8.0 but it looks to me like this still happens in 38.5.1. I checked by pip installing the latest setuptools into the tox environment and running:
$PYTHONWARNINGS="error::DeprecationWarning,error::FutureWarning,default::FutureWarning:pandas._version,default::DeprecationWarning:pip.pep425tags,default::DeprecationWarning:pip.wheel" /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/bin/pip install /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/pep425tags.py:260: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
import imp
Processing /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip
Complete output from command python setup.py egg_info:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/__init__.py", line 14, in <module>
from setuptools.dist import Distribution, Feature
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/dist.py", line 22, in <module>
from setuptools.depends import Require
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/depends.py", line 2, in <module>
import imp
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/imp.py", line 33, in <module>
DeprecationWarning, stacklevel=2)
DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /private/var/folders/8l/psdmt62d0wzfhdr6lc4fvyb00000gn/T/pip-x9uit55z-build/
(py36-full) (error-on-warnings-in-tests *) /Users/davidchudzicki/hypothesis-python
$/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/bin/python
python python3 python3.6
(py36-full) (error-on-warnings-in-tests *) /Users/davidchudzicki/hypothesis-python
$/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/bin/python
Python 3.6.1 (default, Feb 11 2018, 11:43:24)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import setuptools
>>> setuptools.__version__
'38.5.1'
(fixed in latest setuptools) DeprecationWarning: Flags not at the start of the expression build/.*Z(?ms)
Ignored via default::DeprecationWarning:setuptools.command.egg_info.
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/tools/tox --recreate -e py36-full
GLOB sdist-make: /Users/davidchudzicki/hypothesis-python/setup.py
py36-full recreate: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full
py36-full installdeps: -rrequirements/test.txt
py36-full inst: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip
ERROR: invocation failed (exit code 1), logfile: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/log/py36-full-2.log
ERROR: actionid: py36-full
msg: installpkg
cmdargs: ['/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/bin/pip', 'install', '/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip']
Processing /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip
Complete output from command python setup.py egg_info:
running egg_info
creating pip-egg-info/hypothesis.egg-info
writing pip-egg-info/hypothesis.egg-info/PKG-INFO
writing dependency_links to pip-egg-info/hypothesis.egg-info/dependency_links.txt
writing entry points to pip-egg-info/hypothesis.egg-info/entry_points.txt
writing requirements to pip-egg-info/hypothesis.egg-info/requires.txt
writing top-level names to pip-egg-info/hypothesis.egg-info/top_level.txt
writing manifest file 'pip-egg-info/hypothesis.egg-info/SOURCES.txt'
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/depends.py:2: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
import imp
/private/var/folders/8l/psdmt62d0wzfhdr6lc4fvyb00000gn/T/pip-mdk1ksrv-build/setup.py:39: UserWarning: This version of setuptools is too old to correctly store conditional dependencies in binary wheels. For more info, see: https://hynek.me/articles/conditional-python-dependencies/
'This version of setuptools is too old to correctly store '
warning: manifest_maker: standard file '-c' not found
reading manifest file 'pip-egg-info/hypothesis.egg-info/SOURCES.txt'
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/private/var/folders/8l/psdmt62d0wzfhdr6lc4fvyb00000gn/T/pip-mdk1ksrv-build/setup.py", line 122, in <module>
long_description=open(README).read(),
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/distutils/core.py", line 148, in setup
dist.run_commands()
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/distutils/dist.py", line 955, in run_commands
self.run_command(cmd)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/distutils/dist.py", line 974, in run_command
cmd_obj.run()
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/command/egg_info.py", line 279, in run
self.find_sources()
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/command/egg_info.py", line 306, in find_sources
mm.run()
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/command/egg_info.py", line 536, in run
self.prune_file_list()
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/command/egg_info.py", line 576, in prune_file_list
self.filelist.prune(build.build_base)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/command/egg_info.py", line 450, in prune
match = translate_pattern(os.path.join(dir, '**'))
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/setuptools/command/egg_info.py", line 120, in translate_pattern
return re.compile(pat + r'\Z(?ms)')
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/re.py", line 233, in compile
return _compile(pattern, flags)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/re.py", line 301, in _compile
p = sre_compile.compile(pattern, flags)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/sre_compile.py", line 562, in compile
p = sre_parse.parse(p, flags)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/sre_parse.py", line 856, in parse
p = _parse_sub(source, pattern, flags & SRE_FLAG_VERBOSE, False)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/sre_parse.py", line 415, in _parse_sub
itemsappend(_parse(source, state, verbose))
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/sre_parse.py", line 743, in _parse
DeprecationWarning, stacklevel=7
DeprecationWarning: Flags not at the start of the expression build\/.*\Z(?ms)
----------------------------------------
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/pep425tags.py:260: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
import imp
Command "python setup.py egg_info" failed with error code 1 in /private/var/folders/8l/psdmt62d0wzfhdr6lc4fvyb00000gn/T/pip-mdk1ksrv-build/
py36-full installed: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pip/pep425tags.py:260: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses, import imp,apipkg==1.4,attrs==17.4.0,execnet==1.5.0,flaky==3.4.0,mock==2.0.0,pbr==3.1.1,pluggy==0.6.0,py==1.5.2,pytest==3.4.1,pytest-forked==0.2,pytest-xdist==1.22.1,six==1.11.0
___________________________________ summary ____________________________________
ERROR: py36-full: InvocationError: /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/bin/pip install /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip (see /Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/log/py36-full-2.log)
pytest uses deprecated imp module (there's an existing issue)
Issue: https://github.com/pytest-dev/pytest/issues/1403 -- labeled "help wanted", which is cool
Ignored via default::DeprecationWarning:_pytest.assertion.rewrite.
pytest 3.4.1 (latest)
+ python -m pytest -n2 --runpytest=subprocess tests/pytest
Traceback (most recent call last):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pytest.py", line 9, in <module>
from _pytest.config import (
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/_pytest/config.py", line 16, in <module>
import _pytest.assertion
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/_pytest/assertion/__init__.py", line 9, in <module>
from _pytest.assertion import rewrite
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/_pytest/assertion/rewrite.py", line 6, in <module>
import imp
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/imp.py", line 33, in <module>
DeprecationWarning, stacklevel=2)
DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
pytest uses deprecated convert (I filed an issue, closed now b/c the fix is already in the the branch for the next feature release)
Issue: https://github.com/pytest-dev/pytest/issues/3280
Ignored via default::DeprecationWarning:_pytest.fixtures.
pytest 3.4.1
+ python -m pytest -n2 --runpytest=subprocess tests/pytest
/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/_pytest/assertion/rewrite.py:6: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
import imp
Traceback (most recent call last):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/pytest.py", line 13, in <module>
from _pytest.fixtures import fixture, yield_fixture
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/_pytest/fixtures.py", line 840, in <module>
class FixtureFunctionMarker(object):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/_pytest/fixtures.py", line 842, in FixtureFunctionMarker
params = attr.ib(convert=attr.converters.optional(tuple))
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/attr/_make.py", line 153, in attrib
DeprecationWarning, stacklevel=2
DeprecationWarning: The `convert` argument is deprecated in favor of `converter`. It will be removed after 2019/01.
import pandas tries to use deprecated pandas.json (filed an issue)
Issue: https://github.com/pandas-dev/pandas/issues/19944
hypothesis uses invalid escape sequence in docstring (fixed in my PR)
Fixed in: https://github.com/HypothesisWorks/hypothesis-python/pull/1149
I'm happy I at least caught one problem of ours!
This was a real issue in our code unrelated to any other packages:
Traceback (most recent call last):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/versions/python3.6/lib/python3.6/runpy.py", line 85, in _run_code
...
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py36-full/lib/python3.6/site-packages/hypothesis/strategies.py", line 230
"""
SyntaxError: invalid escape sequence \*
Something else to think about with the article:
Suggesting that people run with -Werror in CI seems a little like asking volunteer open source contributors to do more free work. (In most cases I wouldn't actually suggest that internal corporate code, or anyone else fairly far downstream in the dependency graph, do this.)
I think there's a good article to write that doesn't isn't problematic in that way, but it does take some care, I think.
(Edit: Or maybe this isn't a big deal. Any suggestion about how to write better software is a bit like this, so I think as long as I don't write it in a way that doesn't sound like I think I'm entitled to have other people check for warnings in CI, then we're fine.)
I would very much like such a post to exist, because I’m currently having this discussion at work and I’ve been unable to convince myself that it’s definitely a good idea!
Some brief comments:
I think the value of -Werror depends on your language. Compiled languages like C or Scala probably see more benefit than Python.
It’s the sort of thing that’s much easier if you turn it on at the start of your project, than trying to retrofit later. And sometimes even that’s not enough – witness shenanigans above with site.py.
I just went through the process of retrofitting -Werror with a year-old Scala repository (see https://github.com/wellcometrust/platform/pull/1615). I found a few deprecations, one naming confusion, and some other small things – but nothing especially serious or bad.
(Turning this on by fiat was somewhat naughty, and it's not guaranteed we won't revert the change later!)
My feeling is that this works best if the entire ecosystem buys into it. Otherwise single projects will end up swimming against errors in upstream.
@alexwlchan:
I don't know the details of your work, but I'd be surprised if it's a good idea. 😄 I would not encourage anyone to try this at my work.
easier if you turn it on at the start of your project
https://github.com/HypothesisWorks/hypothesis-python/pull/1149 was indeed a fair amount work, but I think it gets us to where we'd be if we had this from the beginning. 😄
best if the entire ecosystem buys into it
Absolutely. Until then (and as the only way to get there), I think it's more sensible the further upstream a project is. I do think we probably are relatively far upstream in that sense (although less so than pip, distutils, setuptools, pytest, ....).
Are you concerned it might not be a good idea for hypothesis / are you opposed to https://github.com/HypothesisWorks/hypothesis-python/pull/1149?
As I mentioned above, a middle ground could be doing this in a way that applies this to only the tests themselves, and avoids the warnings from the build tools. That would have avoided about half the issues I found (although it feels like more than that -- I think the build tools ones were more work to e.g. report with a minimal test case).
I guess I'd prefer to go with what we have now, but I'm open to reworking the PR to something more like that, if that's the consensus.
I don't know the details of your work, but I'd be surprised if it's a good idea. 😄 I would not encourage anyone to try this at my work.
When has “that’s not a good idea” ever stopped me? 😜
Are you concerned it might not be a good idea for hypothesis / are you opposed to #1149?
I’ll be honest, I haven’t had much time to keep up with hypothesis lately, so I haven’t read much of the surrounding discussion. I just caught mention of a possible blog post in the slew of emails, and jumped in!
One error would indeed be fixed by upgrading setuptools, but I chose to ignore it instead: I think (?) our setuptools version is coming from the default py36 tox environment, and if we want to change that I think we should do it in a separate PR.
Fair enough - I'll definitely need to update setuptools for #1091, to store conditional dependencies in install_requires. Happy to leave it until then.
It'd be pretty cool if there were a way to only get deprecation warnings coming directly from the package we're calling, and not ones that are just passed through the package we call.
Looks like pytest can do that - we'd just have to sprinkle python -m -Werror pytest -Werror ... through tox.ini. Unfortunately we can't just put the incantation in an env variable, because the Windows syntax is different. Note also that this only takes effect after warnings filters, so we'd still need some environment tweaks or the double-Werror above (ie we'd dodge build and install issues, but not pytest issues).
Looks like pytest can do that ... Unfortunately
Hmm, maybe keep what we have in https://github.com/HypothesisWorks/hypothesis-python/pull/1149 then (once I get it passing CI, which shouldn't take too much more)?
I’ll be honest, I haven’t had much time to keep up with hypothesis lately,
@alexwlchan No problem, I just wanted to give you the chance to object if you wanted to. If you'll want to think about it in the next week or so, we could also hold off for that?
The downside here isn't that large I suppose. If we ever want to give up on this, it's easy to take it out.
Certainly don’t wait on my behalf, my bandwidth for OSS is fairly small right now.
There's also https://github.com/joke2k/faker/pull/718. This one's a bit frustrating because (I think) it's possible to ignore the way the others can be ignored, since it ends up raising SyntaxError.
Makes it tempting to use pytest -Werror (or the [pytest] section in tox.ini) since I think that would ignore this kind of thing, but that would make me said because it wouldn't have caught the one issue we had in our own code.
The place I'm currently a bit stuck is: In our py34 and py35 environments, using tox-venv gets us very old versions of pip and setuptools.
The py36 environment has new versions, and so do py34 and py35 unless we use tox-env.
Pinning new versions of pip and setuptools in requirements/test.txt makes this problem go away (make check-py34 passes), but pip-compile refuses to transfer those requirements from .in to .txt, telling me it's dangerous to pin those packages.
Possible paths forward:
pip and setuptools that we do, and fixing ittox-venv for those environmentsSorry this has been so long a process. @Zac-HD, if you're tired of it, feel free to let me know and I'll stop. 😄
I think "fixing it" (ie installing the latest versions of pip, setuptools, and wheel) is the right way to go. We can (see comment and docs) use a multi-line install command for that:
install_command =
pip install --upgrade pip setuptools wheel
pip install {opts} {packages}
I'll need to do this for #1091 anyway, but very happy for you to beat me to it! Finally - I'm happy to keep going for as long as you are :smile:
Hey @Zac-HD -- my interpretation of the comment about a multiline install_command is just that it's asking whether that would address the issue if we had it. Not that we have it already.
This matches my experience of trying to use it:
tox.ConfigError: ConfigError: 'install_command' must contain '{packages}' substitution
The only workaround I see is the one suggested at the bottom of the original issue:
Considering that doing the installation manually, using
commands =works
Does this seem right?
(Edit to clarify: This would mean using skip_install=True, and putting all installations in the commdands = section.)
Ah, I misread that then. skip_install isn't my preferred workaround though:
install_command = pip install --upgrade pip setuptools wheel && pip install {opts} {packages}
should localise errors where they belong (though... I haven't tried this either).
Nope, should've mentioned I tried that too:
cmdargs: ['/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/bin/pip', 'install', '--upgrade', 'pip', 'setuptools', 'wheel', '&&', 'pip', 'install', '/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/dist/hypothesis-3.47.0.zip']
You are using pip version 6.0.8, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Exception:
Traceback (most recent call last):
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/lib/python3.4/site-packages/pip/basecommand.py", line 232, in main
status = self.run(options, args)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/lib/python3.4/site-packages/pip/commands/install.py", line 305, in run
name, None, isolated=options.isolated_mode,
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/lib/python3.4/site-packages/pip/req/req_install.py", line 181, in from_line
isolated=isolated)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/lib/python3.4/site-packages/pip/req/req_install.py", line 54, in __init__
req = pkg_resources.Requirement.parse(req)
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/lib/python3.4/site-packages/pip/_vendor/pkg_resources/__init__.py", line 2873, in parse
reqs = list(parse_requirements(s))
File "/Users/davidchudzicki/.cache/hypothesis-build-runtimes/.tox/py34-full/lib/python3.4/site-packages/pip/_vendor/pkg_resources/__init__.py", line 2807, in parse_requirements
raise ValueError("Missing distribution spec", line)
ValueError: ('Missing distribution spec', '&&')
Got any more? 😄
Nope, skip_install it is then, unless the Tox issue turns up something useful :disappointed:
So @Zac-HD, it looks like skip_install may require some large-ish changes in tox.ini. At least some of the current failures on https://github.com/HypothesisWorks/hypothesis-python/pull/1149/commits/c01b5cb8a90c6278b5462a9792d6adf56b0cdaef (e.g. ModuleNotFoundError: No module named 'hypothesis' in pytest28) are because the installs I added to [testenv] don't apply to the environments that define their own commands. So I think we'd have to add installs to all of those too, or something similar.
That seems like it could have all sorts of additional maintenance burden, and doesn't seem worth it just for this, especially because we can really get pretty much what we want by just running basic-test.sh with the environment variables we want after everything is installed.
But are we going to need the skip_install approach for https://github.com/HypothesisWorks/hypothesis-python/issues/1091 anyway? That might change things a bit, but I don't know enough about the problem in https://github.com/HypothesisWorks/hypothesis-python/issues/1091 to know what we'll end up needing.
Even then, PYTHONWARNINGS=... at the tox level has caused so much trouble that I think I'm inclined toward closing https://github.com/HypothesisWorks/hypothesis-python/pull/1149/commits and switching to a new PR that only sets PYTHONWARNINGS=... in basic-test.sh (maybe only in the $PYTEST --runpytest=subprocess tests/pytest line of it, too).
I'd love to hear what you think, though.
Ugh. I suggest we make passenv, setenv, and FutureWarning work as intended, then leave the rest until later.
This saga is dragging out rather badly, and I'd rather get a decent solution in place now, fix #1091 (which may or may not involve skip_install, I'll find out when I do it), and then we can revisit after Hypothesis 4.0 is released and things have been simplified a little (for example, it's removing the Faker extra).
👍