Skip to content
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

REF: insert self.on column _after_ concat #35746

Merged
merged 2 commits into from
Aug 20, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 21 additions & 29 deletions pandas/core/window/rolling.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from pandas.core.base import DataError, PandasObject, SelectionMixin, ShallowMixin
import pandas.core.common as com
from pandas.core.construction import extract_array
from pandas.core.indexes.api import Index, MultiIndex, ensure_index
from pandas.core.indexes.api import Index, MultiIndex
from pandas.core.util.numba_ import NUMBA_FUNC_CACHE, maybe_use_numba
from pandas.core.window.common import (
WindowGroupByMixin,
Expand Down Expand Up @@ -402,36 +402,27 @@ def _wrap_results(self, results, blocks, obj, exclude=None) -> FrameOrSeries:
return result
final.append(result)

# if we have an 'on' column
# we want to put it back into the results
# in the same location
columns = self._selected_obj.columns
if self.on is not None and not self._on.equals(obj.index):

name = self._on.name
final.append(Series(self._on, index=obj.index, name=name))

if self._selection is not None:

selection = ensure_index(self._selection)

# need to reorder to include original location of
# the on column (if its not already there)
if name not in selection:
columns = self.obj.columns
indexer = columns.get_indexer(selection.tolist() + [name])
columns = columns.take(sorted(indexer))

# exclude nuisance columns so that they are not reindexed
if exclude is not None and exclude:
columns = [c for c in columns if c not in exclude]
exclude = exclude or []
columns = [c for c in self._selected_obj.columns if c not in exclude]
if not columns and not len(final) and exclude:
raise DataError("No numeric types to aggregate")
elif not len(final):
return obj.astype("float64")

if not columns:
raise DataError("No numeric types to aggregate")
df = concat(final, axis=1).reindex(columns=columns, copy=False)

if not len(final):
return obj.astype("float64")
return concat(final, axis=1).reindex(columns=columns, copy=False)
# if we have an 'on' column we want to put it back into
# the results in the same location
if self.on is not None and not self._on.equals(obj.index):
name = self._on.name
extra_col = Series(self._on, index=obj.index, name=name)
if name not in df.columns and name not in df.index.names:
new_loc = len(df.columns)
df.insert(new_loc, name, extra_col)
elif name in df.columns:
# TODO: sure we want to overwrite results?
Copy link
Member Author

Choose a reason for hiding this comment

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

@mroeschke can you comment on why we do this?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't really touched _wrap_results much so can't really speak to why we do this. What's an example where this is overwriting results?

Copy link
Member Author

Choose a reason for hiding this comment

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

i dont have an example on hand, but can find one bc i know this line is covered by tests. ATM im inclined to not-worry about it

df[name] = extra_col
return df

def _center_window(self, result, window) -> np.ndarray:
"""
Expand Down Expand Up @@ -2277,6 +2268,7 @@ def _get_window_indexer(self, window: int) -> GroupbyRollingIndexer:
if isinstance(self.window, BaseIndexer):
rolling_indexer = type(self.window)
indexer_kwargs = self.window.__dict__
assert isinstance(indexer_kwargs, dict)
# We'll be using the index of each group later
indexer_kwargs.pop("index_array", None)
elif self.is_freq_type:
Expand Down