-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
Added test test_datetimeField_after_setitem for issue #6942 #28790
Changes from all commits
187ad13
80c52d6
4e02832
93f0385
259013a
768d553
a4db260
9cbf34c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from copy import deepcopy | ||
from datetime import datetime | ||
from distutils.version import LooseVersion | ||
from operator import methodcaller | ||
|
||
|
@@ -280,3 +281,30 @@ def test_deepcopy_empty(self): | |
empty_frame_copy = deepcopy(empty_frame) | ||
|
||
self._compare(empty_frame_copy, empty_frame) | ||
|
||
def test_datetime_after_setitem_with_at(self): | ||
# This test covers the unexpected behaviour of datetimeField when using | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. datetimeField -> datetime64 column ? |
||
# setitem on another column as reported in issue #6942 | ||
|
||
start = pd.to_datetime("20140401") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you use pd.Timestamp here |
||
|
||
result = pd.DataFrame( | ||
index=pd.date_range(start, periods=1), columns=["timenow", "Live"] | ||
) | ||
|
||
result.at[start, "timenow"] = datetime.today() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the "at" call being tested this one or the one below? if not this one, can you set this with a better-tested indexer (or just directly in the constructor call above) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jbrockmendel yeah that could be done. I did that just to replicate the situation in the original issue thread as it mentioned that the "at" worked before setting an entire column(i.e the "Live" column) but not afterwards. |
||
|
||
new_datetime = datetime.today() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it matter that this datetime.today() is called separately from the one two lines up? if not, pls use just one datetime object |
||
|
||
result.Live = True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does the test rely on this being result.Live instead of result["Live"]? if not, pls use the latter |
||
|
||
# Changing the value in "timenow" column after "Live" colunn is set to True. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. colunn -> column |
||
result.at[start, "timenow"] = new_datetime | ||
|
||
expected = pd.DataFrame( | ||
[[new_datetime, True]], | ||
index=pd.date_range(start, periods=1), | ||
columns=["timenow", "Live"], | ||
) | ||
|
||
tm.assert_frame_equal(result, expected) |
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.
this should go in tests.indexing.test_scalar
is "at" the function being tested? if so, the name should be something like "test_at_with..."