Skip to content

parametrize more tests, move to test_liboffstes #18346

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
Nov 19, 2017

Conversation

jbrockmendel
Copy link
Member

Some tests currently in test_offsets belong in test_liboffsets, this moves them.

There is a test_shift_month in test_offsets and another in test_liboffsets. This combines them.

Continuing the process of making the big lists of tests in test_offsets use pytest.mark.parametrize. Is there any downside to doing this? e.g. when it collects the tests, will it hold them all in memory (instead of as a generator)?

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

@jreback
Copy link
Contributor

jreback commented Nov 17, 2017

Continuing the process of making the big lists of tests in test_offsets use pytest.mark.parametrize. Is there any downside to doing this? e.g. when it collects the tests, will it hold them all in memory (instead of as a generator)?

no

the point is usuability, you can run very specific tests, and see exactly what fails, while exercising lots of cases.

these are silghtly more abstruse than a loop, but the above point wins.

@jreback jreback added Frequency DateOffsets Testing pandas testing functions or related to the test suite labels Nov 17, 2017
@jreback
Copy link
Contributor

jreback commented Nov 17, 2017

in some cases you might want to use ids (with a callable) to actually name the tests more intelligently. This is sometimes nicer.

@jbrockmendel
Copy link
Member Author

I didn't think so, but it was worth asking. At one point with dateutil I tried testing every combination of several parameters and RAM usage spiked during pytest test collection, had to kill the process. This was a loop that would have been harmless (albeit poorly advised for other reasons) using older nose-yield-style tests. Let me a little gunshy.

@codecov
Copy link

codecov bot commented Nov 17, 2017

Codecov Report

Merging #18346 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18346      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.02%     
==========================================
  Files         164      164              
  Lines       49790    49790              
==========================================
- Hits        45501    45492       -9     
- Misses       4289     4298       +9
Flag Coverage Δ
#multiple 89.17% <ø> (ø) ⬆️
#single 39.53% <ø> (-0.03%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️

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 cfad581...90e4c80. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 17, 2017

Codecov Report

Merging #18346 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18346      +/-   ##
==========================================
- Coverage   91.38%   91.36%   -0.02%     
==========================================
  Files         164      164              
  Lines       49790    49790              
==========================================
- Hits        45501    45492       -9     
- Misses       4289     4298       +9
Flag Coverage Δ
#multiple 89.17% <ø> (ø) ⬆️
#single 39.53% <ø> (-0.03%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️

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 cfad581...90e4c80. Read the comment docs.

@jreback jreback added this to the 0.22.0 milestone Nov 19, 2017
@jreback jreback merged commit f724066 into pandas-dev:master Nov 19, 2017
@jreback
Copy link
Contributor

jreback commented Nov 19, 2017

thanks!

@jbrockmendel jbrockmendel deleted the tslibs-offsets-tests3 branch December 8, 2017 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frequency DateOffsets Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants