-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: allow to check for specific xlrd version and skip reader test if xlrd < 0.9 #5058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fine with me. At some point probably better to use LooseVersion, but seems like a good fix. |
TST: allow to check for specific xlrd version and skip reader test if xlrd < 0.9
# If above test passes with outdated xlrd, next test | ||
# does require fresh xlrd | ||
# http://nipy.bic.berkeley.edu/builders/pandas-py2.x-wheezy-sparc/builds/148/steps/shell_4/logs/stdio | ||
_skip_if_no_xlrd((0, 9)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should have asked this before I merged, but what does this actually get you? Aren't you using the same exact version as before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bleh -- indeed! ;) sorry about that... here though it would specify version explicitly ;)
probably default version in _skip_if_no_xlrd could be downgraded somewhat. as that url from buildbot shows -- previous test passed fine with 0.6 version but next one failed with it. but that would need more testing... I might check tomorrow on that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize that's why you wanted to do that. You can't even find xlrd 0.6 on PyPI anymore (or at least I can't find it). But I guess that's what on Debian stable :-/. We'd have to discuss supporting xlrd 0.6 (because it'd also need to be a dep for one of the builds).
I would be -1 on different minimum versions for different parts of tests. Others? @jreback @cpcloud ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yarikoptic just an FYI you can compare versions much easier with the distutils.versions.LooseVersion
class:
In [8]: from distutils.version import LooseVersion
In [9]: np.__version__ == LooseVersion('1.6.1')
Out[9]: True
In [10]: np.__version__ == LooseVersion('1.6')
Out[10]: False
In [11]: np.__version__ >= LooseVersion('1.6')
Out[11]: True
I'm also -1 on different versions of things for different tests, although i do something kind of like this for read_html
(basically if bs4 version is 4.2.0 then i raise, but that's only tested in one place and i skip tests otherwise)
it would be ideal to have all of our tests support a single minimum version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, 02 Oct 2013, Jeff Tratner wrote:
Oh, I didn't realize that's why you wanted to do that. You can't even find
xlrd 0.6 on PyPI anymore (or at least I can't find it). But I guess that's
what on Debian stable :-/.
not only Debian stable:
$> whohas -D Debian,Ubuntu --strict python-xlrd | sort
Debian python-xlrd 0.6.1-2 119K all http://packages.debian.org/squeeze/python-xlrd
Debian python-xlrd 0.6.1-2 119K all http://packages.debian.org/wheezy/python-xlrd
Debian python-xlrd 0.9.2-1 140K all http://packages.debian.org/jessie/python-xlrd
Debian python-xlrd 0.9.2-1 140K all http://packages.debian.org/sid/python-xlrd
Ubuntu python-xlrd 0.6.1-2 118K all http://packages.ubuntu.com/lucid/python-xlrd
Ubuntu python-xlrd 0.6.1-2 118K all http://packages.ubuntu.com/precise/python-xlrd
Ubuntu python-xlrd 0.6.1-2 118K all http://packages.ubuntu.com/quantal/python-xlrd
Ubuntu python-xlrd 0.6.1-2 118K all http://packages.ubuntu.com/raring/python-xlrd
Ubuntu python-xlrd 0.9.2-1 140K all http://packages.ubuntu.com/saucy/python-xlrd
Yaroslav O. Halchenko, Ph.D.
http://neuro.debian.net http://www.pymvpa.org http://www.fail2ban.org
Senior Research Associate, Psychological and Brain Sciences Dept.
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, you must have monkey-patched Excel, right? ExcelFile mandates xlrd >= 0.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, 02 Oct 2013, Phillip Cloud wrote:
with ExcelWriter(pth) as writer: self.frame.to_excel(writer, 'Data1') self.frame2.to_excel(writer, 'Data2')
# If above test passes with outdated xlrd, next test
# does require fresh xlrd
# http://nipy.bic.berkeley.edu/builders/pandas-py2.x-wheezy-sparc/builds/148/steps/shell_4/logs/stdio
_skip_if_no_xlrd((0, 9))
[1]@yarikoptic just an FYI you can compare versions much more easily with
the distutils.versions.LooseVersion class:
thanks -- I know it well ;) I just came up with the minimalistic patch
here. So +1 to replace that tuple comparison with LooseVersion(version)
comparison
I'm also -1 on different versions of things for different tests, although
i do something kind of like this for read_html (basically if bs4 version
is 4.2.0 then i raise, but that's only tested in one place and i skip
tests otherwise)
btw -- while at it... you might like how we 'centralized' version
checking in PyMVPA:
versions "dict" gets populated upon request for a given module:
https://github.com/PyMVPA/PyMVPA/blob/HEAD/mvpa2/base/externals.py
and then we have a generic helper (not per module e.g. _skip_if_no_xlrd):
skip_if_no_external('mdp', min_version='2.5')
which also could have max_version .
Cheers
it would be ideal to have all of our tests support a single minimum
version
well, imho it is ok sooner or later for some tests support new functionality of
external modules while older ones being as good for whatever uses only
old goodies
Yaroslav O. Halchenko, Ph.D.
http://neuro.debian.net http://www.pymvpa.org http://www.fail2ban.org
Senior Research Associate, Psychological and Brain Sciences Dept.
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to keep posting here, but the entire test suite fails if you try to use 0.6.1 (after monkey-patching to remove all the checks for xlrd). Please tell us what you mean here @yarikoptic .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, 02 Oct 2013, Jeff Tratner wrote:
In pandas/io/tests/test_excel.py:
@@ -350,7 +350,10 @@ def test_excelwriter_contextmanager(self):
with ExcelWriter(pth) as writer:
self.frame.to_excel(writer, 'Data1')self.frame2.to_excel(writer, 'Data2')
# If above test passes with outdated xlrd, next test
# does require fresh xlrd
# http://nipy.bic.berkeley.edu/builders/pandas-py2.x-wheezy-sparc/builds/148/steps/shell_4/logs/stdio
_skip_if_no_xlrd((0, 9))
Sorry to keep posting here, but the entire test suite fails if you try to
use 0.6.1 (after monkey-patching to remove all the checks for xlrd).
Please tell us what you mean here [1]@yarikoptic .
I just wanted to address failure as in
http://nipy.bic.berkeley.edu/builders/pandas-py2.x-wheezy-sparc/builds/148/steps/shell_4/logs/stdio
which happened somehow here after preceding test... thus I assumed that
it might work ok with 0.6.1 ... may be it doesn't -- didn't check.
so whatever works for you -- works for me ;-)
Yaroslav O. Halchenko, Ph.D.
http://neuro.debian.net http://www.pymvpa.org http://www.fail2ban.org
Senior Research Associate, Psychological and Brain Sciences Dept.
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yarikoptic that's because the skip test is missing here :P let's change that instead.
minimum version I can get to pass the test suite is 0.8. |
See http://nipy.bic.berkeley.edu/builders/pandas-py2.x-wheezy-sparc/builds/148/steps/shell_4/logs/stdio for failure when outdated xlrd found (e.g. on debian stable wheezy)