-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
#14854: DatetimeIndex compiled together in test_datetime.py #15266
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
#14854: DatetimeIndex compiled together in test_datetime.py #15266
Conversation
Codecov Report@@ Coverage Diff @@
## master #15266 +/- ##
=======================================
Coverage 86.33% 86.33%
=======================================
Files 139 139
Lines 51149 51149
=======================================
Hits 44157 44157
Misses 6992 6992Continue to review full report at Codecov.
|
|
ok, so the resulting test file is just too big and unweildy. let's split it out to a sub-dir, IOW
then split into a number of files (you can start by splitting each class to a separate named file for instance) |
|
nice start! name things like
|
|
Thanks. Though, What about the questions I raised or is this PR ready? |
no |
essentially yes these should be moved. |
|
certainly would like to split up test_datetime.py Ideally split along class lines (though maybe not much there, it might already be a big class). So can come back and do that later. |
|
also need to add an entry in |
|
Now only remaining thing, it seems, is the merging of those 2 different function definitions by same name ( |
|
@TrigonaMinima sure, let's get this passing. Then pls make a list of what changes you think could / should be made. We want to move and then in other commits, fix/change etc. |
|
@jreback I am sorry, I didn't understand you. Changes like? Merging those two functions and what else? Also, other commits under this PR only, right? |
|
Okay so here's the list I came up with
|
|
@TrigonaMinima if you can get this passing (iow fix the flake errors) would be great. |
|
@jreback the build in Python2.7 is giving errors in all the tests in Any suggestions? Changing the name of the directory is the easiest fix I can think of. |
|
@TrigonaMinima change the directory name to datetimes I think will work |
|
FYI you can do
|
|
Yeah. I read the full contributing guide yesterday. Though, this module name problem wasn't showing up in the output of the command. Which is the right behavior given this isn't exactly a pep8 issue. |
|
@TrigonaMinima yeah, for some reason travis catches slightly different things that local flake8, not really sure why (its only this particular pep, missing a whole module import) |
| def test_round(self): | ||
|
|
||
| # round | ||
| dt = Timestamp('20130101 09:10:11') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is something to add to the list.
I want to create a pandas/tests/scalars/test_timestamp.py (and test_timedelta.py). So anything specifically Timestamp related will go there.
this can be next iteration though.
| class TestDatetimeIndex(tm.TestCase): | ||
| _multiprocess_can_split_ = True | ||
|
|
||
| def test_construction_with_alt(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so ok with splitting this up a bit now. e.g. test_construction (and when you do, copy the class (name), and leave the methods alone (even though they say test_construction_* for now that's ok))
| freq='B') | ||
| tm.assert_index_equal(result, expected) | ||
|
|
||
| def test_astype(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_astype.py
| self.assertRaises(ValueError, idx.astype, 'datetime64') | ||
| self.assertRaises(ValueError, idx.astype, 'datetime64[D]') | ||
|
|
||
| def test_where_other(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_indexing.py
| tm.assert_index_equal(result, exp) | ||
| self.assertEqual(result.freq, 'D') | ||
|
|
||
| def test_fillna_datetime64(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_missing.py
| dtype=object) | ||
| self.assert_index_equal(idx.fillna('x'), exp) | ||
|
|
||
| def test_difference_freq(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_setops. (include union, intersection, difference routines)
|
|
||
| self.assertEqual(idx.nanosecond[0], t1.nanosecond) | ||
|
|
||
| def test_constructor_coverage(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another one for test_construction
| non_datetime = Index(list('abc')) | ||
| self.assertFalse(idx.equals(list(non_datetime))) | ||
|
|
||
| def test_union_coverage(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to test_setops
| self.assertEqual(result.name, df.index[2]) | ||
|
|
||
| # GH 10699 | ||
| def test_datetime64_with_DateOffset(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can go to test_ops
| self.assertTrue(result.freq is None) | ||
| self.assertEqual(result.tz, expected.tz) | ||
|
|
||
| def test_delete(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put insert and delete into test_indexing
| self.assertEqual(result.freq, expected.freq) | ||
| self.assertEqual(result.tz, expected.tz) | ||
|
|
||
| def test_take(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take -> test_indexing
…atetime.py (GH14854)
…test_datetime.py (GH14854)
|
thanks @TrigonaMinima ! ok I am going to push a scalar directory now that is empty, but please continue on moving things! |
xref pandas-dev#14854 Author: TrigonaMinima <[email protected]> Closes pandas-dev#15266 from TrigonaMinima/issue-14854-datetime and squashes the following commits: 6ee2bd9 [TrigonaMinima] TST: Splitting test_datetime.py into smaller chunks (gh14854) 415a748 [TrigonaMinima] TST: Moving DatetimeIndex related tests from test_timeseries.py and flake8 fixes c43c7de [TrigonaMinima] TST: proper naming of files 458d141 [TrigonaMinima] TST: splitting test_datetime.py 1ff0819 [TrigonaMinima] TST: fix flake8 errors - test_datetime.py (GH14854) 9311161 [TrigonaMinima] TST: reorg of DatetimeIndex tests from tseries/tests/test_base.py to test_datetime.py (GH14854) 54421a5 [TrigonaMinima] TST: reorg of DatetimeIndex tests from test_datetimelike.py to test_datetime.py (GH14854) f83814b [TrigonaMinima] TST: reorg of DatetimeIndex tests from test_timeseries.py to test_datetime.py
Partially completes #14854
Grouped all the
DatetimeIndexrelated tests under one file -pandas/tests/indexes/test_datetime.py.Tests reorganized
There were some tests in other classes which could be shifted to the as well. From
pandas/pandas/tseries/tests/test_timeseries.pyTestTimeSeriesTestDatetime64TestDatetime64TestDatetime64TestDatetime64Should these also be included in the
test_datetime.pyin their respective classes or let them be?In
pandas/tests/indexes/test_datetimelike.pythere were some tests which were using theDatetimeLikeclass defined there. I could import that class in thetest_datetime.pyand transfer the wholeTestDatetimeIndexbut should it be done?Mostly passes
git diff upstream/master | flake8 --difftest_intersectionandtest_unionfunctions?Further TODOs from the following discussion and the #14854 under datetime
Split
datetimes/test_datetime.pyinto smaller chunksIn
datetimes/test_misc.pylines from 167-202 should go under timestamp related tests (?)Transfer of function (
test_datetimeindex_accessors) from classTestDatetime64indatetimes/test_misc.pyto classTestDatetimeIndexOpsofdatetime/test_ops.py(?)Function
test_roundfromdatetimes/test_datetime.pytopandas/tests/scalars/test_timestamp.py. (and test_timedelta.py). So anything specifically Timestamp related will go there.