Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 2, 2013
+7 −4
Merged

TST: allow to check for specific xlrd version and skip reader test if xlrd < 0.9 #5058

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pandas/io/tests/test_excel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
import pandas as pd


def _skip_if_no_xlrd():
def _skip_if_no_xlrd(version=(0, 9)):
try:
import xlrd
ver = tuple(map(int, xlrd.__VERSION__.split(".")[:2]))
if ver < (0, 9):
raise nose.SkipTest('xlrd < 0.9, skipping')
if ver < version:
raise nose.SkipTest('xlrd < %s, skipping' % str(version))
except ImportError:
raise nose.SkipTest('xlrd not installed, skipping')

Expand Down Expand Up @@ -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))
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

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 ?

Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

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 .

Copy link
Contributor Author

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

Copy link
Contributor

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.

with ExcelFile(pth) as reader:
found_df = reader.parse('Data1')
found_df2 = reader.parse('Data2')
Expand Down