-
-
Notifications
You must be signed in to change notification settings - Fork 19.1k
Updating generic.py error message #8618
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 2 commits
589098c
2e3c35b
f41ef3d
168b83a
6cc47ca
968d68f
6bf83c5
e5fe75e
0ef5c07
1e5d25a
9fd99d7
c8e36d4
a3e478d
3b2089b
cfcda5f
99a7318
f21539b
26129f5
e9ee47c
fef4b09
e2f8f0a
2e59e42
e759d99
8290a4d
bcaf7fd
d55f582
b1168b5
ca70dbe
11c53a7
baa8cd6
ab1c90f
e40215a
49bd373
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 |
---|---|---|
|
@@ -2868,9 +2868,17 @@ def groupby(self, by=None, axis=0, level=None, as_index=True, sort=True, | |
""" | ||
|
||
from pandas.core.groupby import groupby | ||
axis = self._get_axis_number(axis) | ||
return groupby(self, by, axis=axis, level=level, as_index=as_index, | ||
sort=sort, group_keys=group_keys, squeeze=squeeze) | ||
if axis is not None: | ||
axis = self._get_axis_number(axis) | ||
return groupby(self, by, axis=axis, level=level, as_index=as_index, | ||
sort=sort, group_keys=group_keys, squeeze=squeeze) | ||
elif level is not None: | ||
raise TypeError('You have to specify one of "by" or "level"') | ||
elif by is not None: | ||
raise TypeError('You have to specify one of "by" or "level"') | ||
else: | ||
raise TypeError('You have to specify one of "by" or "level"') | ||
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 don't really understand the logic here. You first test for 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. Any suggestions on improving this logic? 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 not fully sure, but something like just checking that at least level or by is specified:
and then just proceed with how it was original? 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 tried that and then I get broken tests. Any suggestions? FAIL: test_fails_on_no_datetime_index (pandas.tseries.tests.test_resample.TestTimeGrouper) Traceback (most recent call last): FAIL: test_insert_error_msmgs (pandas.tests.test_frame.TestDataFrame)Traceback (most recent call last): FAIL: test_filter_enforces_scalarness (pandas.tests.test_groupby.TestGroupBy)Traceback (most recent call last): FAIL: test_filter_non_bool_raises (pandas.tests.test_groupby.TestGroupBy)Traceback (most recent call last): 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. Hmm, difficult to say without seeing the code. Can you just push your latest attempt? 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 nosetests fail but sure :) 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. Should be all pushed now :) |
||
|
||
|
||
def asfreq(self, freq, method=None, how=None, normalize=False): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1961,6 +1961,9 @@ def test_groupby_level(self): | |
# raise exception for non-MultiIndex | ||
self.assertRaises(ValueError, self.df.groupby, level=1) | ||
|
||
|
||
|
||
|
||
def test_groupby_level_index_names(self): | ||
## GH4014 this used to raise ValueError since 'exp'>1 (in py2) | ||
df = DataFrame({'exp' : ['A']*3 + ['B']*3, 'var1' : lrange(6),}).set_index('exp') | ||
|
@@ -1999,6 +2002,27 @@ def test_groupby_level_apply(self): | |
result = frame['A'].groupby(level=0).count() | ||
self.assertEqual(result.index.name, 'first') | ||
|
||
|
||
#PR8618 and issue 8015 | ||
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 align this with |
||
def test_groupby_args(self): | ||
frame = self.mframe | ||
def g(): | ||
frame.groupby(level=None).count() | ||
self.assertRaisesRegexp(TypeError, g, "You have to supply one of 'by' or 'level'") | ||
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 assert should not be in the function definition, as in this way it is not called:
|
||
|
||
def k(): | ||
frame.groupby(by=None).count() | ||
self.assertRaisesRegexp(TypeError, k, "You have to supply one of 'by' or 'level'") | ||
|
||
def j(): | ||
frame.groupby() | ||
self.assertRaisesRegexp(TypeError, j, "You have to supply one of 'by' or 'level'") | ||
|
||
def i(): | ||
frame.groupby(axes=None) | ||
self.assertRaisesRegexp(TypeError, i, "You have to supply one of 'by' or 'level'") | ||
|
||
|
||
def test_groupby_level_mapper(self): | ||
frame = self.mframe | ||
deleveled = frame.reset_index() | ||
|
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.
For what reason are the two
elif
s included? Because iflevel
is not None (it means that level is specified), is should not raise, no? (but you do raise here)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.
You are right I shouldn't raise for not none I should just raise for None.