MAINT: Complete Conversion to Pytest Idiom #16201

gfyoung · 2017-05-02T16:31:20Z

Convert all test setup/teardown to pytest idiom
Remove any classes that inherit from unittest.TestCase
Update documentation that we are still using tm.TestCase

pytest idiom for test setup/teardown can be found here.

gfyoung · 2017-05-02T16:57:35Z

@jreback : This test failure here makes no sense to me at all. I can't reproduce this when I walk through the test (though it replicates when I run the tests locally). Any idea as to why this is happening?

jreback · 2017-05-02T18:35:06Z

/home/travis/build/pandas-dev/pandas/pandas/tests/frame/test_query_eval.py yield tests are deprecated, and scheduled to be removed in pytest 4.0

gfyoung · 2017-05-02T18:36:04Z

@jreback : I don't really follow. What's the point of this comment?

jreback · 2017-05-02T19:19:29Z

that shouldn't happen, that means a fixture is being used incorrectly.

gfyoung · 2017-05-02T19:20:56Z

Yeah, there is some kind of cache poisoning somewhere, as when I check IntervalDtype._cache before the assertion, it already has None mapped to interval64.

jreback · 2017-05-02T19:22:04Z

hmm, that could be. so you can relax / eliminate that test then. prob has to do with ordering of the cache insertions.

gfyoung · 2017-05-02T19:52:34Z

But that cache insertion should never have happened though. I'm surprised that this only surfaced now (don't really see how my change would have caused it).

gfyoung · 2017-05-02T21:01:22Z

Something goes wrong when you call round_trip_pickle. You can replicate here:

>>> from pandas.core.dtypes.dtypes import IntervalDtype
>>> import pandas.util.testing as tm
>>>
>>> dtype = IntervalDtype("int64")
>>> IntervalDtype._cache
{'int64': interval[int64]}
>>>
>>> none_dtype = IntervalDtype("interval")
>>> IntervalDtype._cache
{'int64': interval[int64], 'None': interval}
>>>
>>> result = tm.round_trip_pickle(dtype)
>>> IntervalDtype._cache  # what!?
{'int64': interval[int64], 'None': interval[int64]}

A closer look indicates that there is aliasing going on in the cache. That's why the value mutates so mysteriously, which must happen when the pickled object is reloaded. This makes sense given that we are ~~deserializing~~ unpickling, so we must have instantiated an empty IntervalDtype and then set the attributes for the instance.

gfyoung · 2017-05-02T21:32:07Z

@jreback : What was the rationale for having a _cache attribute (this type of construction seems particular to just these duck-typed dtype)? Is it possible that we can just remove this caching given that this interferes with pickling?

The other option is that we add a copy parameter to the constructor, so that we can return copies and avoid this kind of aliasing.

jreback · 2017-05-02T21:53:21Z

i will fix
the cache is important here
it shouldn't be serializing though

gfyoung · 2017-05-02T21:54:12Z

Sorry, I should have said "unpickling," but okay, I will await a patch for the aliasing then.

jreback · 2017-05-03T00:54:25Z

FYI merging #16207 shortly. you can pick up pandas/tests/dtypes/test_dtypes.py from master as I fixed this for pytest.

xref #16201

codecov · 2017-05-03T03:00:57Z

Codecov Report

Merging #16201 into master will decrease coverage by 0.63%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #16201      +/-   ##
==========================================
- Coverage   90.86%   90.23%   -0.64%     
==========================================
  Files         162      162              
  Lines       50884    50883       -1     
==========================================
- Hits        46237    45914     -323     
- Misses       4647     4969     +322

Flag	Coverage Δ
#multiple	`88.01% <100%> (-0.64%)`	⬇️
#single	`40.31% <100%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/util/testing.py	`78.86% <100%> (-0.21%)`	⬇️
pandas/io/formats/excel.py	`73.15% <0%> (-23.49%)`	⬇️
pandas/io/excel.py	`61.68% <0%> (-18.94%)`	⬇️
pandas/core/tools/datetimes.py	`66.94% <0%> (-18.62%)`	⬇️
pandas/core/config.py	`69.72% <0%> (-18.37%)`	⬇️
pandas/core/indexes/base.py	`95.75% <0%> (-0.46%)`	⬇️
pandas/compat/__init__.py	`61.77% <0%> (-0.45%)`	⬇️
pandas/io/parsers.py	`95.32% <0%> (-0.33%)`	⬇️
pandas/core/indexes/datetimes.py	`95.23% <0%> (-0.2%)`	⬇️
pandas/core/series.py	`94.8% <0%> (-0.19%)`	⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae70ece...51eab8c. Read the comment docs.

codecov · 2017-05-03T03:00:59Z

Codecov Report

Merging #16201 into master will decrease coverage by 0.62%.
The diff coverage is 70%.

@@            Coverage Diff             @@
##           master   #16201      +/-   ##
==========================================
- Coverage   90.86%   90.24%   -0.63%     
==========================================
  Files         162      162              
  Lines       50887    50882       -5     
==========================================
- Hits        46240    45916     -324     
- Misses       4647     4966     +319

Flag	Coverage Δ
#multiple	`88.02% <70%> (-0.63%)`	⬇️
#single	`40.31% <50%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/util/testing.py	`78.97% <70%> (-0.1%)`	⬇️
pandas/io/formats/excel.py	`73.15% <0%> (-23.49%)`	⬇️
pandas/io/excel.py	`61.68% <0%> (-18.94%)`	⬇️
pandas/core/tools/datetimes.py	`66.94% <0%> (-18.62%)`	⬇️
pandas/core/config.py	`69.72% <0%> (-18.37%)`	⬇️
pandas/core/indexes/base.py	`95.75% <0%> (-0.46%)`	⬇️
pandas/compat/__init__.py	`61.77% <0%> (-0.45%)`	⬇️
pandas/io/parsers.py	`95.32% <0%> (-0.33%)`	⬇️
pandas/core/series.py	`94.8% <0%> (-0.19%)`	⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1002cc3...7bed86a. Read the comment docs.

gfyoung · 2017-05-03T06:38:33Z

@jreback : Circle is failing (see here), and I can't reproduce it. It seems like there is some kind of weird matplotlib import failure that shouldn't be happening, but I have no idea why it is happening (can't reproduce locally with and without matplotlib installed).

One thought is that we can try using pytest.importorskip functionality, but that doesn't explain why Circle is failing in the first place when Travis and Appveyor are not.

jreback · 2017-05-03T10:38:04Z

not sure. you should try rolling back parts of the change until this work.

* tm.TestCase now just inherits from object * setUpClass renamed to setup_class * tearDownClass renamed to teardown_class * setUp renamed to setup_method * tearDown renamed to teardown_method

tm.TestCase no longer follows the nosetest idiom, so it is here to stay, so update the documentation to say that we are using it still. Closes pandas-devgh-15990.

The tm.mplskip decorator was breaking on Circle, so this commit removes the decorator and replaces it with direct function calls to check for matplotlib.

gfyoung · 2017-05-04T01:11:08Z

in another PR can then remove tm.TestCase from the base class of test class where it is clearly not needed

The only thing that tm.TestCase contributes is raising on chained assignment (part of setup_class). I presume that is to just discourage tests from being written like this. Could we just remove that (and just make sure to be vigilant about it when writing tests)?

jreback · 2017-05-04T01:38:26Z

The only thing that tm.TestCase contributes is raising on chained assignment (part of setup_class). I presume that is to just discourage tests from being written like this. Could we just remove that (and just make sure to be vigilant about it when writing tests)?

NO, this is the entire point of this. as I said you can remove from everywhere except pandas/tests/frame & pandas/tests/indexing

jreback · 2017-05-04T01:39:03Z

and just make sure to be vigilant about it when writing tests

people are NEVER vigilant, assume that they aim to break every single rule :>

gfyoung · 2017-05-04T01:42:41Z

But the default for chained assignment is "warning" - would that not surface in the build logs if tests were incorrectly written with chained assignment?

jreback · 2017-05-04T02:02:34Z

pandas/tests/frame/test_query_eval.py

        tm.skip_if_no_ne(engine)
+        skip_if_no_pandas_parser(parser)


you can actually do this with a fixture skip, but no big deal really

jreback · 2017-05-04T02:08:51Z

ping when green. I have a PR after this one (moving stuff around).

jreback · 2017-05-04T02:09:12Z

But the default for chained assignment is "warning" - would that not surface in the build logs if tests were incorrectly written with chained assignment?

no its not for testing, its raise.

gfyoung · 2017-05-04T02:10:15Z

no its not for testing, its raise.

But that's because you set it that way when you set up each test class. I meant in general, it's warning by default, hence my question.

gfyoung · 2017-05-04T02:33:23Z

@jreback : All green and ready to go.

jreback · 2017-05-04T02:38:09Z

But that's because you set it that way when you set up each test class. I meant in general, it's warning by default, hence my question.

but that's exactly the point. if someone writes a test that uses chained indexing I WANT it to fail the tests. Sure we can look for it, but its best to automatically catch it.

jreback · 2017-05-04T02:38:19Z

thanks for this.

gfyoung · 2017-05-04T02:42:01Z

but that's exactly the point. if someone writes a test that uses chained indexing I WANT it to fail the tests.

What if we added it to conftest.py (i.e. just add the line pd.set_option(...))?

jreback · 2017-05-04T03:01:03Z

hmm that might work actually

gfyoung · 2017-05-04T03:03:07Z

I just gave it a shot, and I think it might work (here is code snippet)

test_example.txt

I'll make a PR for replace tm.TestCase then.

jreback · 2017-05-04T03:13:55Z

ok sounds good

…s-dev#16207) xref pandas-dev#16201

* MAINT: Convert test setup/teardown to pytest idiom * tm.TestCase now just inherits from object * setUpClass renamed to setup_class * tearDownClass renamed to teardown_class * setUp renamed to setup_method * tearDown renamed to teardown_method * MAINT: Remove unittest.TestCase from testing * DOC: Update documentation for TestCase usage tm.TestCase no longer follows the nosetest idiom, so it is here to stay, so update the documentation to say that we are using it still. Closes pandas-devgh-15990. * TST: Patch Circle matplotlib failure The tm.mplskip decorator was breaking on Circle, so this commit removes the decorator and replaces it with direct function calls to check for matplotlib. * TST: Replace yield-based tests in test_query_eval

Version 0.20.0 * tag 'v0.20.0': (742 commits) RLS: v0.20.0 DOC: Whatsnew cleanup (pandas-dev#16245) TST: Test CategoricalIndex in test_is_categorical (pandas-dev#16243) TST: xfail some bottleneck on windows (pandas-dev#16240) DOC, TST: Document and Test Functions in dtypes/common.py (pandas-dev#16237) TST: Remove __init__ statements in testing (pandas-dev#16238) DOC: don't include all methods/attributes of IntervalIndex (pandas-dev#16221) PKG: Fix ModuleNotFoundError: No module named 'pandas.formats' (pandas-dev#16239) RLS: v0.20.0rc2 CLN: make submodules of pandas.util private (pandas-dev#16223) MAINT: Remove tm.TestCase from testing (pandas-dev#16225) MAINT: Complete Conversion to Pytest Idiom (pandas-dev#16201) DOC: add whatsnew for 0.21.0 DEPR: correct deprecation message for datetools (pandas-dev#16202) API Change repr name for table schema (pandas-dev#16204) DOC: Remove various warnings from doc build (pandas-dev#16206) DOC: add whatsnew for v0.20.1 BUG: Fixed renaming of falsey names in build_table_schema (pandas-dev#16205) COMPAT: ensure proper extension dtype's don't pickle the cache (pandas-dev#16207) REF: register custom DisplayFormatter for table schema (pandas-dev#16198) ...

gfyoung changed the title ~~MAINT: Complete Conversion to PyTest Idiom~~ MAINT: Complete Conversion to Pytest Idiom May 2, 2017

gfyoung force-pushed the unittest-class-remove branch from f688e6b to 96ca3be Compare May 2, 2017 18:02

jreback added the Testing pandas testing functions or related to the test suite label May 2, 2017

gfyoung force-pushed the unittest-class-remove branch 2 times, most recently from 4e00445 to 2ba0a8f Compare May 2, 2017 21:38

jreback mentioned this pull request May 2, 2017

COMPAT: ensure proper extension dtype's don't pickle the cache #16207

Merged

jreback added a commit that referenced this pull request May 3, 2017

COMPAT: ensure proper extension dtype's don't pickle the cache (#16207)

154a647

xref #16201

gfyoung force-pushed the unittest-class-remove branch 2 times, most recently from 94bd4ab to 51eab8c Compare May 3, 2017 03:00

gfyoung force-pushed the unittest-class-remove branch from 51eab8c to fa7bdd9 Compare May 3, 2017 05:31

gfyoung force-pushed the unittest-class-remove branch 2 times, most recently from 84ad896 to fa7bdd9 Compare May 3, 2017 08:00

gfyoung force-pushed the unittest-class-remove branch 2 times, most recently from fbfa7c2 to 7ec9853 Compare May 3, 2017 16:17

gfyoung added 4 commits May 3, 2017 20:55

MAINT: Convert test setup/teardown to pytest idiom

515f58a

* tm.TestCase now just inherits from object * setUpClass renamed to setup_class * tearDownClass renamed to teardown_class * setUp renamed to setup_method * tearDown renamed to teardown_method

MAINT: Remove unittest.TestCase from testing

bdc0a89

DOC: Update documentation for TestCase usage

e0284e4

tm.TestCase no longer follows the nosetest idiom, so it is here to stay, so update the documentation to say that we are using it still. Closes pandas-devgh-15990.

TST: Patch Circle matplotlib failure

0fb4854

The tm.mplskip decorator was breaking on Circle, so this commit removes the decorator and replaces it with direct function calls to check for matplotlib.

TST: Replace yield-based tests in test_query_eval

7bed86a

gfyoung force-pushed the unittest-class-remove branch from 9608307 to 7bed86a Compare May 4, 2017 01:23

jreback reviewed May 4, 2017

View reviewed changes

jreback approved these changes May 4, 2017

View reviewed changes

jreback added this to the 0.20.0 milestone May 4, 2017

jreback merged commit 4a748cc into pandas-dev:master May 4, 2017

gfyoung deleted the unittest-class-remove branch May 4, 2017 02:49

gfyoung mentioned this pull request May 4, 2017

MAINT: Remove tm.TestCase from testing #16225

Merged

pcluo pushed a commit to pcluo/pandas that referenced this pull request May 22, 2017

COMPAT: ensure proper extension dtype's don't pickle the cache (panda…

55325b2

…s-dev#16207) xref pandas-dev#16201

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: Complete Conversion to Pytest Idiom #16201

MAINT: Complete Conversion to Pytest Idiom #16201

gfyoung commented May 2, 2017 •

edited

Loading

gfyoung commented May 2, 2017 •

edited

Loading

jreback commented May 2, 2017

gfyoung commented May 2, 2017

jreback commented May 2, 2017

gfyoung commented May 2, 2017

jreback commented May 2, 2017

gfyoung commented May 2, 2017

gfyoung commented May 2, 2017 •

edited

Loading

gfyoung commented May 2, 2017

jreback commented May 2, 2017

gfyoung commented May 2, 2017 •

edited

Loading

jreback commented May 3, 2017

codecov bot commented May 3, 2017

codecov bot commented May 3, 2017 •

edited

Loading

gfyoung commented May 3, 2017 •

edited

Loading

jreback commented May 3, 2017

gfyoung commented May 4, 2017

jreback commented May 4, 2017

jreback commented May 4, 2017

gfyoung commented May 4, 2017

jreback May 4, 2017

jreback commented May 4, 2017

jreback commented May 4, 2017

gfyoung commented May 4, 2017 •

edited

Loading

gfyoung commented May 4, 2017

jreback commented May 4, 2017

jreback commented May 4, 2017

gfyoung commented May 4, 2017 •

edited

Loading

jreback commented May 4, 2017

gfyoung commented May 4, 2017 •

edited

Loading

jreback commented May 4, 2017

MAINT: Complete Conversion to Pytest Idiom #16201

MAINT: Complete Conversion to Pytest Idiom #16201

Conversation

gfyoung commented May 2, 2017 • edited Loading

gfyoung commented May 2, 2017 • edited Loading

jreback commented May 2, 2017

gfyoung commented May 2, 2017

jreback commented May 2, 2017

gfyoung commented May 2, 2017

jreback commented May 2, 2017

gfyoung commented May 2, 2017

gfyoung commented May 2, 2017 • edited Loading

gfyoung commented May 2, 2017

jreback commented May 2, 2017

gfyoung commented May 2, 2017 • edited Loading

jreback commented May 3, 2017

codecov bot commented May 3, 2017

Codecov Report

codecov bot commented May 3, 2017 • edited Loading

Codecov Report

gfyoung commented May 3, 2017 • edited Loading

jreback commented May 3, 2017

gfyoung commented May 4, 2017

jreback commented May 4, 2017

jreback commented May 4, 2017

gfyoung commented May 4, 2017

jreback May 4, 2017

Choose a reason for hiding this comment

jreback commented May 4, 2017

jreback commented May 4, 2017

gfyoung commented May 4, 2017 • edited Loading

gfyoung commented May 4, 2017

jreback commented May 4, 2017

jreback commented May 4, 2017

gfyoung commented May 4, 2017 • edited Loading

jreback commented May 4, 2017

gfyoung commented May 4, 2017 • edited Loading

jreback commented May 4, 2017

gfyoung commented May 2, 2017 •

edited

Loading

gfyoung commented May 2, 2017 •

edited

Loading

gfyoung commented May 2, 2017 •

edited

Loading

gfyoung commented May 2, 2017 •

edited

Loading

codecov bot commented May 3, 2017 •

edited

Loading

gfyoung commented May 3, 2017 •

edited

Loading

gfyoung commented May 4, 2017 •

edited

Loading

gfyoung commented May 4, 2017 •

edited

Loading

gfyoung commented May 4, 2017 •

edited

Loading