Skip to content

BUG:Time Grouper bug fix when applied for list groupers #17587

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Oct 1, 2017
Merged

BUG:Time Grouper bug fix when applied for list groupers #17587

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6838d9c
use grouper if given the list if grouper
ruiann Sep 18, 2017
66febe4
BinGrouper holds the sorted binners, give the indexer to reorder axis…
ruiann Sep 19, 2017
0ff386f
PEP8 fix
ruiann Sep 19, 2017
00a18e3
PEP8 & comment
ruiann Sep 20, 2017
cc1a4ac
comment
ruiann Sep 22, 2017
6927207
new method for BaseGrouper and inherited class
ruiann Sep 25, 2017
3ce56bd
comment
ruiann Sep 26, 2017
737c00e
move test to test time grouper
ruiann Sep 28, 2017
b868cbc
update whatsnew entry
ruiann Sep 29, 2017
950de20
fixups
jreback Oct 1, 2017
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
BinGrouper holds the sorted binners, give the indexer to reorder axis… 
… if needed, eg. build grouping labels
  • Loading branch information
@ruiann @jreback
ruiann authored and jreback committed Oct 1, 2017
commit 66febe4e54260f0875526fa193423360715e2421
9 changes: 7 additions & 2 deletions pandas/core/groupby.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1730,13 +1730,14 @@ class BaseGrouper(object):
"""

def __init__(self, axis, groupings, sort=True, group_keys=True,
mutated=False):
mutated=False, indexer=None):
self._filter_empty_groups = self.compressed = len(groupings) != 1
Copy link
Contributor

@jreback jreback Sep 19, 2017

Choose a reason for hiding this comment

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

can you add a doc-string explaining params (I know you just added 1 but good time)
Copy link
Contributor Author

@ruiann ruiann Sep 20, 2017

Choose a reason for hiding this comment

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

I'm sorry I don't really understand all parameters, I've added those I know😂
self.axis = axis
self.groupings = groupings
self.sort = sort
self.group_keys = group_keys
self.mutated = mutated
self.indexer = indexer

@property
def shape(self):
Expand Down Expand Up @@ -2282,11 +2283,12 @@ def generate_bins_generic(values, binner, closed):

class BinGrouper(BaseGrouper):

def __init__(self, bins, binlabels, filter_empty=False, mutated=False):
def __init__(self, bins, binlabels, filter_empty=False, mutated=False, indexer=None):
self.bins = _ensure_int64(bins)
self.binlabels = _ensure_index(binlabels)
self._filter_empty_groups = filter_empty
self.mutated = mutated
self.indexer = indexer

@cache_readonly
def groups(self):
Expand Down Expand Up @@ -2554,6 +2556,9 @@ def _make_labels(self):
if isinstance(self.grouper, BaseGrouper):
Copy link
Contributor

@jreback jreback Sep 25, 2017

Choose a reason for hiding this comment

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

same
labels, _, _ = self.grouper.group_info
uniques = self.grouper.result_index
if self.grouper.indexer is not None:
Copy link
Contributor

@jreback jreback Sep 21, 2017

Choose a reason for hiding this comment

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

refactor to make a method on the BaseGrouper itself and override in BinGrouper; same for indices
Copy link
Contributor Author

@ruiann ruiann Sep 22, 2017

Choose a reason for hiding this comment

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

I'm afraid cannot use this way. Grouping use the group_info for unsorted axis, while some other scenarios call the group_info to group sorted axis, I think it's better to keep the group_info as sorted, and reorder to get the unsorted label sequence.
Copy link
Contributor

@jreback jreback Sep 22, 2017

Choose a reason for hiding this comment

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

that's fine, but not my point. I don't want these if/else in the properties, rather they should simply be overriden methods on the grouper type
Copy link
Contributor Author

@ruiann ruiann Sep 25, 2017

Choose a reason for hiding this comment

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

OK I get your point.
sorter = np.lexsort((labels, self.grouper.indexer))
labels = labels[sorter]
else:
labels, uniques = algorithms.factorize(
self.grouper, sort=self.sort)
Expand Down
11 changes: 9 additions & 2 deletions pandas/core/resample.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def _get_binner(self):
"""

binner, bins, binlabels = self._get_binner_for_time()
bin_grouper = BinGrouper(bins, binlabels)
bin_grouper = BinGrouper(bins, binlabels, indexer=self.groupby.indexer)
return binner, bin_grouper

def _assure_grouper(self):
Expand Down Expand Up @@ -879,7 +879,14 @@ def _downsample(self, how, **kwargs):

if is_subperiod(ax.freq, self.freq):
# Downsampling
return self._groupby_and_aggregate(how, grouper=self.grouper)
if len(new_index) == 0:
bins = []
else:
i8 = memb.asi8
rng = np.arange(i8[0], i8[-1] + 1)
bins = memb.searchsorted(rng, side='right')
grouper = BinGrouper(bins, new_index, indexer=self.groupby.indexer)
return self._groupby_and_aggregate(how, grouper=grouper)
elif is_superperiod(ax.freq, self.freq):
if how == 'ohlc':
# GH #13083
Expand Down