Skip to content

TST/DOC: Add procedure for TestPickle #9401

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

Merged
merged 1 commit into from
Feb 11, 2015
Merged

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Feb 3, 2015

Added procedure to TestPickle. Based on #9291, I think updating setup.py is likely to be ommitted.

@jreback
Copy link
Contributor

jreback commented Feb 4, 2015

yep. Is there any way to have setup automatically pick up all of the directories under a tree in package_data ?

@jreback jreback added Docs Deprecate Functionality to remove in pandas and removed Deprecate Functionality to remove in pandas labels Feb 4, 2015
@jreback jreback added this to the 0.16.0 milestone Feb 4, 2015
@sinhrks
Copy link
Member Author

sinhrks commented Feb 4, 2015

@jreback Yeah it's better. Modified both setup.py and test_pickle.py to test all files under "legacy_pickle".

self.read_pickles('0.14.0')
n += 1
assert n > 0, 'Pickle files are not tested'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than making these in one test, can you have a routine which create tests from the legacy_pickle dir (like you are doing now), but makes them separate tests (so need to do this at import time). That way you can see them running indepedently (and if they fail they would fail independetly)

@sinhrks sinhrks force-pushed the pickle_test branch 2 times, most recently from af85543 to ac487ad Compare February 5, 2015 13:37
@sinhrks
Copy link
Member Author

sinhrks commented Feb 5, 2015

OK. I understand that to make nose to find dynamically added test methods, we have to use test generators. Thus, I changed the TestPickle not to inherit unittest.TestCase. If there is a better way, please lmk.
http://stackoverflow.com/questions/6689537/nose-test-generators-inside-class

When execute the test, we will see:

__main__.TestPickle.test_pickles('0.10.1',) ... ok
__main__.TestPickle.test_pickles('0.11.0',) ... ok
__main__.TestPickle.test_pickles('0.12.0',) ... ok
__main__.TestPickle.test_pickles('0.13.0',) ... ok
__main__.TestPickle.test_pickles('0.14.0',) ... ok
__main__.TestPickle.test_pickles('0.14.1',) ... ok
__main__.TestPickle.test_pickles('0.15.0',) ... ok
__main__.TestPickle.test_pickles('0.15.2',) ... ok
__main__.TestPickle.test_round_trip_current ... ok

----------------------------------------------------------------------
Ran 9 tests in 1.359s

OK

@@ -18,7 +18,17 @@
from pandas.util.misc import is_little_endian
import pandas

class TestPickle(tm.TestCase):
class TestPickle():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put a comment why we are not sub-classing TestCase here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@jreback
Copy link
Contributor

jreback commented Feb 5, 2015

@sinhrks ok

a possible 'soln' to this is to make make all of the methods in tm.TestCase (currently) into a MixIn (and include it there, so nothing changes). And then you can use the MixIn here.

@sinhrks
Copy link
Member Author

sinhrks commented Feb 8, 2015

@jreback Based on #8923, better to use methods under tm... without creating mixin? I've modified test cases to use them.

jreback added a commit that referenced this pull request Feb 11, 2015
TST/DOC: Add procedure for TestPickle
@jreback jreback merged commit 726e892 into pandas-dev:master Feb 11, 2015
@jreback
Copy link
Contributor

jreback commented Feb 11, 2015

thanks for this!

@sinhrks sinhrks deleted the pickle_test branch February 15, 2015 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants