From fa4b4ef23cae006ba45f793dfcffb4a671e296ca Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 26 Sep 2019 20:32:45 -0700 Subject: [PATCH 1/2] CLN: Define and pin GroupBy properties without exec --- ci/code_checks.sh | 6 ++- pandas/core/groupby/generic.py | 85 +++++++++++++++++++--------------- pandas/core/groupby/groupby.py | 2 - 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/ci/code_checks.sh b/ci/code_checks.sh index b03c4f2238445..e13738b98833a 100755 --- a/ci/code_checks.sh +++ b/ci/code_checks.sh @@ -125,6 +125,10 @@ if [[ -z "$CHECK" || "$CHECK" == "patterns" ]]; then # invgrep -R --include="*.py*" -E "from numpy import nan " pandas # GH#24822 not yet implemented since the offending imports have not all been removed RET=$(($RET + $?)) ; echo $MSG "DONE" + MSG='Check for use of exec' ; echo $MSG + invgrep -R --include="*.py*" -E "[^a-zA-Z0-9_]exec\(" pandas + RET=$(($RET + $?)) ; echo $MSG "DONE" + MSG='Check for pytest warns' ; echo $MSG invgrep -r -E --include '*.py' 'pytest\.warns' pandas/tests/ RET=$(($RET + $?)) ; echo $MSG "DONE" @@ -184,7 +188,7 @@ if [[ -z "$CHECK" || "$CHECK" == "patterns" ]]; then invgrep -R --include="*.rst" ".. ipython ::" doc/source RET=$(($RET + $?)) ; echo $MSG "DONE" - MSG='Check that no file in the repo contains tailing whitespaces' ; echo $MSG + MSG='Check that no file in the repo contains trailing whitespaces' ; echo $MSG set -o pipefail if [[ "$AZURE" == "true" ]]; then # we exclude all c/cpp files as the c/cpp files of pandas code base are tested when Linting .c and .h files diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index f8f1455561c03..9cca76c7b440b 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -11,7 +11,7 @@ from functools import partial from textwrap import dedent import typing -from typing import Any, Callable, FrozenSet, Iterator, Sequence, Type, Union +from typing import Any, Callable, FrozenSet, Sequence, Type, Union import warnings import numpy as np @@ -70,47 +70,64 @@ ScalarResult = typing.TypeVar("ScalarResult") -def whitelist_method_generator( - base_class: Type[GroupBy], klass: Type[FrameOrSeries], whitelist: FrozenSet[str] -) -> Iterator[str]: +def generate_property(name: str, klass: Type[FrameOrSeries]): """ - Yields all GroupBy member defs for DataFrame/Series names in whitelist. + Create a property for a GroupBy subclass to dispatch to DataFrame/Series. + + Parameters + ---------- + name : str + klass : {DataFrame, Series} + + Returns + ------- + property + """ + + @property + def prop(self): + return self._make_wrapper(name) + + parent_method = getattr(klass, name) + prop.__doc__ = parent_method.__doc__ or "" + prop.fget.__name__ = name + return prop + + +def pin_whitelisted_properties(klass: Type[FrameOrSeries], whitelist: FrozenSet[str]): + """ + Create GroupBy member defs for DataFrame/Series names in a whitelist. Parameters ---------- - base_class : Groupby class - base class klass : DataFrame or Series class class where members are defined. - whitelist : frozenset + whitelist : frozenset[str] Set of names of klass methods to be constructed Returns ------- - The generator yields a sequence of strings, each suitable for exec'ing, - that define implementations of the named methods for DataFrameGroupBy - or SeriesGroupBy. + class decorator + Notes + ----- Since we don't want to override methods explicitly defined in the base class, any such name is skipped. """ - property_wrapper_template = """@property -def %(name)s(self) : - \"""%(doc)s\""" - return self.__getattr__('%(name)s')""" - - for name in whitelist: - # don't override anything that was explicitly defined - # in the base class - if hasattr(base_class, name): - continue - # ugly, but we need the name string itself in the method. - f = getattr(klass, name) - doc = f.__doc__ - doc = doc if type(doc) == str else "" - wrapper_template = property_wrapper_template - params = {"name": name, "doc": doc} - yield wrapper_template % params + + def pinner(cls): + for name in whitelist: + if hasattr(cls, name): + # don't override anything that was explicitly defined + # in the base class + continue + + prop = generate_property(name, klass) + setattr(cls, name, prop) + + return cls + + return pinner class NDFrameGroupBy(GroupBy): @@ -747,13 +764,9 @@ def filter(self, func, dropna=True, *args, **kwargs): return self._apply_filter(indices, dropna) +@pin_whitelisted_properties(Series, base.series_apply_whitelist) class SeriesGroupBy(GroupBy): - # - # Make class defs of attributes on SeriesGroupBy whitelist - _apply_whitelist = base.series_apply_whitelist - for _def_str in whitelist_method_generator(GroupBy, Series, _apply_whitelist): - exec(_def_str) @property def _selection_name(self): @@ -1368,15 +1381,11 @@ def pct_change(self, periods=1, fill_method="pad", limit=None, freq=None): return (filled / shifted) - 1 +@pin_whitelisted_properties(DataFrame, base.dataframe_apply_whitelist) class DataFrameGroupBy(NDFrameGroupBy): _apply_whitelist = base.dataframe_apply_whitelist - # - # Make class defs of attributes on DataFrameGroupBy whitelist. - for _def_str in whitelist_method_generator(GroupBy, DataFrame, _apply_whitelist): - exec(_def_str) - _block_agg_axis = 1 _agg_see_also_doc = dedent( diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index e010e615e176e..f9c8e7748b7f7 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -562,8 +562,6 @@ def __getattr__(self, attr): return object.__getattribute__(self, attr) if attr in self.obj: return self[attr] - if hasattr(self.obj, attr): - return self._make_wrapper(attr) raise AttributeError( "%r object has no attribute %r" % (type(self).__name__, attr) From 5bdd87ad5cde94c6425edb3901bf776264ca177a Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Fri, 27 Sep 2019 07:32:24 -0700 Subject: [PATCH 2/2] mypy fixup --- pandas/core/groupby/generic.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 9cca76c7b440b..0ab19448043f6 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -84,14 +84,13 @@ def generate_property(name: str, klass: Type[FrameOrSeries]): property """ - @property def prop(self): return self._make_wrapper(name) parent_method = getattr(klass, name) prop.__doc__ = parent_method.__doc__ or "" - prop.fget.__name__ = name - return prop + prop.__name__ = name + return property(prop) def pin_whitelisted_properties(klass: Type[FrameOrSeries], whitelist: FrozenSet[str]):