-
-
Notifications
You must be signed in to change notification settings - Fork 18.7k
BUG: Series[timdelta64].var() should _not_ work #28289
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
Changes from 34 commits
704b422
ec02d1f
db1567f
c8ff4a0
2c8ef9a
1ed8a96
8abdbaf
23d2eca
ecc8b36
9f78ec7
3ca8c86
812b5fb
2e09bd6
966f749
18033b4
b9f61cb
c319080
e467976
cb7ef3b
52f4ff1
61feeeb
db73405
010e2ef
04a546a
27fea95
fb0374f
9e401dd
965f0a7
fa1eb8c
58801d3
59b83e1
6687b69
d258e83
5bd457d
f827fb0
619c394
d37fcb3
c753617
66bc863
02a566b
4d3d8ec
94023db
317bd2a
9598c2f
a6c6b87
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 |
---|---|---|
|
@@ -706,11 +706,20 @@ def nanstd(values, axis=None, skipna=True, ddof=1, mask=None): | |
>>> nanops.nanstd(s) | ||
1.0 | ||
""" | ||
orig_dtype = values.dtype | ||
if is_timedelta64_dtype(values.dtype): | ||
# std is well-behaved, but var is not | ||
if mask is None: | ||
mask = isna(values) | ||
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. i think this might be better fixed if we have nanvar call _get_values (which already does this masking & type conversion); its the only routine which doesn't I think (well maybe sem). 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. The problem with that is that we need different behavior for nanstd vs nanvar 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 try to address this 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. Well, no. We have 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. I am concerned about this code duplicatio of _get_values. 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. I'll double check 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. Yep, turns out we can use _get_values. updated |
||
else: | ||
mask = mask | isna(values) | ||
values = values.view("i8") | ||
|
||
result = np.sqrt(nanvar(values, axis=axis, skipna=skipna, ddof=ddof, mask=mask)) | ||
return _wrap_results(result, values.dtype) | ||
return _wrap_results(result, orig_dtype) | ||
|
||
|
||
@disallow("M8") | ||
@disallow("M8", "m8") | ||
@bottleneck_switch(ddof=1) | ||
def nanvar(values, axis=None, skipna=True, ddof=1, mask=None): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4012,6 +4012,13 @@ def _reduce( | |
If we have an ndarray as a value, then simply perform the operation, | ||
otherwise delegate to the object. | ||
""" | ||
if name == "var" and is_timedelta64_dtype(self.dtype): | ||
mroeschke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
raise TypeError( | ||
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 there anyway you can do this in nanops instead? it is very awkward to do here 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. Looks like this is no longer necessary, removed. |
||
"reduction operation '{name}' not allowed for this dtype".format( | ||
name=name | ||
) | ||
) | ||
|
||
delegate = self._values | ||
|
||
if axis is not None: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -300,11 +300,14 @@ def test_timedelta_ops(self): | |
assert result[0] == expected | ||
|
||
# invalid ops | ||
for op in ["skew", "kurt", "sem", "prod"]: | ||
for op in ["skew", "kurt", "sem", "prod", "var"]: | ||
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. let's split this to a separate test that is paramterized (followon) |
||
msg = "reduction operation '{}' not allowed for this dtype" | ||
with pytest.raises(TypeError, match=msg.format(op)): | ||
getattr(td, op)() | ||
|
||
with pytest.raises(TypeError, match=msg.format(op)): | ||
getattr(td.to_frame(), op)(numeric_only=False) | ||
|
||
# GH#10040 | ||
# make sure NaT is properly handled by median() | ||
s = Series([Timestamp("2015-02-03"), Timestamp("2015-02-07")]) | ||
|
@@ -636,8 +639,13 @@ def test_ops_consistency_on_empty(self, method): | |
assert pd.isna(result) | ||
|
||
# timedelta64[ns] | ||
result = getattr(Series(dtype="m8[ns]"), method)() | ||
assert result is pd.NaT | ||
tdser = Series([], dtype="m8[ns]") | ||
if method == "var": | ||
with pytest.raises(TypeError, match="operation 'var' not allowed"): | ||
getattr(tdser, method)() | ||
else: | ||
result = getattr(tdser, method)() | ||
assert result is pd.NaT | ||
|
||
def test_nansum_buglet(self): | ||
ser = Series([1.0, np.nan], index=[0, 1]) | ||
|
Uh oh!
There was an error while loading. Please reload this page.