Skip to content
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

Separate tests specific to tslibs modules #18036

Merged
merged 7 commits into from
Oct 31, 2017

Conversation

jbrockmendel
Copy link
Member

Going through the tests in tests/scalars, tests/indexes/datetimes, tests/indexes/timedeltas, tests/indexes/period, there is a lot of duplication and a lot of thematically inappropriate placement. This is a natural result of tests being added over the years.

One result of this, though, is that its easy to miss certain corner cases (#17991, #7996).

There are two things I'd like to do here. First is gather tests specific to tslibs modules so they can be self-contained. This PR starts that for tslibs.parsing.

Second is to go through tests.indexes centralize arithmetic tests, deduplicate where needed (#18026). This is a big task without an immediate payoff, so I want to get the go-ahead before jumping in.

  • closes #xxxx
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@gfyoung gfyoung added the Testing pandas testing functions or related to the test suite label Oct 30, 2017

result = parsing.try_parse_dates(arr, dayfirst=True)
expected = [parse(d, dayfirst=True) for d in arr]
assert np.array_equal(result, expected)
Copy link
Member

@gfyoung gfyoung Oct 30, 2017

Choose a reason for hiding this comment

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

I know this is a refactoring, but let's take this opportunity to remove np.array_equal from our testing. Let's rewrite to use tm.assert_numpy_array_equal instead.

@codecov
Copy link

codecov bot commented Oct 30, 2017

Codecov Report

Merging #18036 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18036      +/-   ##
==========================================
+ Coverage   91.24%   91.25%   +<.01%     
==========================================
  Files         163      163              
  Lines       50100    50100              
==========================================
+ Hits        45714    45717       +3     
+ Misses       4386     4383       -3
Flag Coverage Δ
#multiple 89.06% <ø> (+0.02%) ⬆️
#single 40.24% <ø> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.75% <0%> (-0.1%) ⬇️
pandas/plotting/_converter.py 65.2% <0%> (+1.81%) ⬆️

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 8449ffd...a99cd6f. Read the comment docs.

@codecov
Copy link

codecov bot commented Oct 30, 2017

Codecov Report

Merging #18036 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18036      +/-   ##
==========================================
+ Coverage   91.24%   91.25%   +<.01%     
==========================================
  Files         163      163              
  Lines       50100    50100              
==========================================
+ Hits        45714    45717       +3     
+ Misses       4386     4383       -3
Flag Coverage Δ
#multiple 89.06% <ø> (+0.02%) ⬆️
#single 40.24% <ø> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.75% <0%> (-0.1%) ⬇️
pandas/plotting/_converter.py 65.2% <0%> (+1.81%) ⬆️

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 8449ffd...beca092. Read the comment docs.

@@ -136,4 +136,4 @@ def test_try_parse_dates(self):

result = parsing.try_parse_dates(arr, dayfirst=True)
expected = [parse(d, dayfirst=True) for d in arr]
assert np.array_equal(result, expected)
assert tm.assert_numpy_array_equal(result, expected)
Copy link
Member

@gfyoung gfyoung Oct 30, 2017

Choose a reason for hiding this comment

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

Remove the assert keyword. tm.assert_... takes care of that for you. As written, this will cause an AssertionError since the function call returns None.

@jreback
Copy link
Contributor

jreback commented Oct 31, 2017

you took out 15 tests and added back 10, what happened?

@jreback jreback added the Datetime Datetime data dtype label Oct 31, 2017
@jbrockmendel
Copy link
Member Author

There was a Travis stall in test_clipboard (I think), needed to re-start CI, so needed to make a new commit. As it happens, tests.indexes.datetimes.test_ops.TestDateTimeIndexToJulianDate is a duplicate of tests.indexes.datetimes.test_misc.TestDateTimeIndexToJulianDate, so I removed the former.

@jreback jreback added this to the 0.22.0 milestone Oct 31, 2017
@jreback jreback merged commit 823cfc4 into pandas-dev:master Oct 31, 2017
@jreback
Copy link
Contributor

jreback commented Oct 31, 2017

thanks!

@jreback
Copy link
Contributor

jreback commented Oct 31, 2017

as far as fully exercising numeric ops. all for it. Most of these are tested directly thru a single inheritance mechnaism revolving around subclassing the mixins in test_base.py You can certainly isolate these and add, but this is quite tricky, and we have to assure that we are not losing coverage.

peterpanmj pushed a commit to peterpanmj/pandas that referenced this pull request Oct 31, 2017
@jbrockmendel jbrockmendel mentioned this pull request Nov 6, 2017
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
@jbrockmendel jbrockmendel deleted the tslibs-testing3 branch December 8, 2017 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants