Skip to content

BUG: Matched Round Signature with NumPy's #12603

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
14 changes: 14 additions & 0 deletions doc/source/whatsnew/v0.18.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ Performance Improvements
Bug Fixes
~~~~~~~~~

- ``round`` now accepts an ``out`` argument to maintain compatibility with numpy's ``round`` function (:issue:`12600`)
- Bug in ``Period`` and ``PeriodIndex`` creation raises ``KeyError`` if ``freq="Minute"`` is specified. Note that "Minute" freq is deprecated in v0.17.0, and recommended to use ``freq="T"`` instead (:issue:`11854`)
- Bug in printing data which contains ``Period`` with different ``freq`` raises ``ValueError`` (:issue:`12615`)
- Bug in ``Series`` construction with ``Categorical`` and ``dtype='category'`` is specified (:issue:`12574`)
Expand All @@ -114,6 +115,18 @@ Bug Fixes


















Expand All @@ -130,6 +143,7 @@ Bug Fixes




- Bug in ``concat`` raises ``AttributeError`` when input data contains tz-aware datetime and timedelta (:issue:`12620`)


Expand Down
8 changes: 7 additions & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from pandas import compat
from pandas.util.decorators import (deprecate, Appender, Substitution,
deprecate_kwarg)
from pandas.util.validators import validate_args

from pandas.tseries.period import PeriodIndex
from pandas.tseries.index import DatetimeIndex
Expand Down Expand Up @@ -4420,7 +4421,7 @@ def merge(self, right, how='inner', on=None, left_on=None, right_on=None,
right_index=right_index, sort=sort, suffixes=suffixes,
copy=copy, indicator=indicator)

def round(self, decimals=0, out=None):
def round(self, decimals=0, *args):
Copy link
Contributor

Choose a reason for hiding this comment

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

are these passed by position or kw?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, numpy passes them in as positional in fromnumeric.py, so if we want to hide out, it needs to reside in an *args parameter and NOT a **kwargs one.

"""
Round a DataFrame to a variable number of decimal places.

Expand Down Expand Up @@ -4471,6 +4472,8 @@ def round(self, decimals=0, out=None):
See Also
--------
numpy.around
Series.round

"""
from pandas.tools.merge import concat

Expand All @@ -4486,6 +4489,9 @@ def _series_round(s, decimals):
return s.round(decimals)
return s

validate_args(args, min_length=0, max_length=1,
msg="Inplace rounding is not supported")

if isinstance(decimals, (dict, Series)):
if isinstance(decimals, Series):
if not decimals.index.is_unique:
Expand Down
25 changes: 5 additions & 20 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
AbstractMethodError)
import pandas.core.nanops as nanops
from pandas.util.decorators import Appender, Substitution, deprecate_kwarg
from pandas.util.validators import validate_kwargs
from pandas.core import config

# goal is to be able to define the docs close to function, while still being
Expand Down Expand Up @@ -5231,29 +5232,13 @@ def _doc_parms(cls):
%(outname)s : %(name1)s\n"""


def _validate_kwargs(fname, kwargs, *compat_args):
"""
Checks whether parameters passed to the
**kwargs argument in a 'stat' function 'fname'
are valid parameters as specified in *compat_args

"""
list(map(kwargs.__delitem__, filter(
kwargs.__contains__, compat_args)))
if kwargs:
bad_arg = list(kwargs)[0] # first 'key' element
raise TypeError(("{fname}() got an unexpected "
"keyword argument '{arg}'".
format(fname=fname, arg=bad_arg)))


def _make_stat_function(name, name1, name2, axis_descr, desc, f):
@Substitution(outname=name, desc=desc, name1=name1, name2=name2,
axis_descr=axis_descr)
@Appender(_num_doc)
def stat_func(self, axis=None, skipna=None, level=None, numeric_only=None,
**kwargs):
_validate_kwargs(name, kwargs, 'out', 'dtype')
validate_kwargs(name, kwargs, 'out', 'dtype')
if skipna is None:
skipna = True
if axis is None:
Expand All @@ -5274,7 +5259,7 @@ def _make_stat_function_ddof(name, name1, name2, axis_descr, desc, f):
@Appender(_num_ddof_doc)
def stat_func(self, axis=None, skipna=None, level=None, ddof=1,
numeric_only=None, **kwargs):
_validate_kwargs(name, kwargs, 'out', 'dtype')
validate_kwargs(name, kwargs, 'out', 'dtype')
if skipna is None:
skipna = True
if axis is None:
Expand All @@ -5296,7 +5281,7 @@ def _make_cum_function(name, name1, name2, axis_descr, desc, accum_func,
@Appender("Return cumulative {0} over requested axis.".format(name) +
_cnum_doc)
def func(self, axis=None, dtype=None, out=None, skipna=True, **kwargs):
_validate_kwargs(name, kwargs, 'out', 'dtype')
validate_kwargs(name, kwargs, 'out', 'dtype')
if axis is None:
axis = self._stat_axis_number
else:
Expand Down Expand Up @@ -5331,7 +5316,7 @@ def _make_logical_function(name, name1, name2, axis_descr, desc, f):
@Appender(_bool_doc)
def logical_func(self, axis=None, bool_only=None, skipna=None, level=None,
**kwargs):
_validate_kwargs(name, kwargs, 'out', 'dtype')
validate_kwargs(name, kwargs, 'out', 'dtype')
if skipna is None:
skipna = True
if axis is None:
Expand Down
7 changes: 6 additions & 1 deletion pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
from pandas.tseries.period import PeriodIndex, Period
from pandas import compat
from pandas.util.terminal import get_terminal_size
from pandas.util.validators import validate_args
from pandas.compat import zip, u, OrderedDict, StringIO


Expand Down Expand Up @@ -1256,7 +1257,7 @@ def idxmax(self, axis=None, out=None, skipna=True):
argmin = idxmin
argmax = idxmax

def round(self, decimals=0):
def round(self, decimals=0, *args):
"""
Round each value in a Series to the given number of decimals.

Expand All @@ -1274,8 +1275,12 @@ def round(self, decimals=0):
See Also
--------
numpy.around
DataFrame.round

"""
validate_args(args, min_length=0, max_length=1,
msg="Inplace rounding is not supported")

result = _values_from_object(self).round(decimals)
result = self._constructor(result, index=self.index).__finalize__(self)

Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/frame/test_analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -2060,6 +2060,17 @@ def test_round(self):
assert_series_equal(df.round(decimals)['col1'],
expected_rounded['col1'])

def test_numpy_round(self):
# See gh-12600
df = DataFrame([[1.53, 1.36], [0.06, 7.01]])
out = np.round(df, decimals=0)
expected = DataFrame([[2., 1.], [0., 7.]])
assert_frame_equal(out, expected)

msg = "Inplace rounding is not supported"
with tm.assertRaisesRegexp(ValueError, msg):
np.round(df, decimals=0, out=df)

def test_round_mixed_type(self):
# GH11885
df = DataFrame({'col1': [1.1, 2.2, 3.3, 4.4],
Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/series/test_analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,17 @@ def test_round(self):
assert_series_equal(result, expected)
self.assertEqual(result.name, self.ts.name)

def test_numpy_round(self):
# See gh-12600
s = Series([1.53, 1.36, 0.06])
out = np.round(s, decimals=0)
expected = Series([2., 1., 0.])
assert_series_equal(out, expected)

msg = "Inplace rounding is not supported"
with tm.assertRaisesRegexp(ValueError, msg):
np.round(s, decimals=0, out=s)

def test_built_in_round(self):
if not compat.PY3:
raise nose.SkipTest(
Expand Down
77 changes: 77 additions & 0 deletions pandas/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import nose

from pandas.util.decorators import deprecate_kwarg
from pandas.util.validators import validate_args, validate_kwargs

import pandas.util.testing as tm


Expand Down Expand Up @@ -73,6 +75,81 @@ def test_rands_array():
assert(arr.shape == (10, 10))
assert(len(arr[1, 1]) == 7)


class TestValidateArgs(tm.TestCase):

def test_bad_min_length(self):
msg = "'min_length' must be non-negative"
with tm.assertRaisesRegexp(ValueError, msg):
validate_args((None,), min_length=-1, max_length=5)

def test_bad_arg_length_no_max(self):
min_length = 5
msg = "expected at least {min_length} arguments".format(
min_length=min_length)

with tm.assertRaisesRegexp(ValueError, msg):
validate_args((None,), min_length=min_length, max_length=None)

def test_bad_arg_length_with_max(self):
min_length = 5
max_length = 10
msg = ("expected between {min_length} and {max_length}"
" arguments inclusive".format(min_length=min_length,
max_length=max_length))

with tm.assertRaisesRegexp(ValueError, msg):
validate_args((None,), min_length=min_length,
max_length=max_length)

def test_bad_min_max_length(self):
msg = "'min_length' > 'max_length'"
with tm.assertRaisesRegexp(ValueError, msg):
validate_args((None,), min_length=5, max_length=2)

def test_not_all_none(self):
msg = "All arguments must be None"
with tm.assertRaisesRegexp(ValueError, msg):
validate_args(('foo',), min_length=0,
max_length=1, msg=msg)

with tm.assertRaisesRegexp(ValueError, msg):
validate_args(('foo', 'bar', 'baz'), min_length=2,
max_length=5, msg=msg)

with tm.assertRaisesRegexp(ValueError, msg):
validate_args((None, 'bar', None), min_length=2,
max_length=5, msg=msg)

def test_validation(self):
# No exceptions should be thrown
validate_args((None,), min_length=0, max_length=1)
validate_args((None, None), min_length=1, max_length=5)


class TestValidateKwargs(tm.TestCase):

def test_bad_kwarg(self):
goodarg = 'f'
badarg = goodarg + 'o'

kwargs = {goodarg: 'foo', badarg: 'bar'}
compat_args = (goodarg, badarg + 'o')
fname = 'func'

msg = ("{fname}\(\) got an unexpected "
"keyword argument '{arg}'".format(
fname=fname, arg=badarg))

with tm.assertRaisesRegexp(TypeError, msg):
validate_kwargs(fname, kwargs, *compat_args)

def test_validation(self):
# No exceptions should be thrown
compat_args = ('f', 'b', 'ba')
kwargs = {'f': 'foo', 'b': 'bar'}
validate_kwargs('func', kwargs, *compat_args)

if __name__ == '__main__':
nose.runmodule(argv=[__file__, '-vvs', '-x', '--pdb', '--pdb-failure'],
exit=False)
97 changes: 97 additions & 0 deletions pandas/util/validators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
"""
Module that contains many useful utilities
for validating data or function arguments
"""


def validate_args(args, min_length=0, max_length=None, msg=""):
"""
Checks whether the length of the `*args` argument passed into a function
has at least `min_length` arguments. If `max_length` is an integer, checks
whether `*args` has at most `max_length` arguments inclusive. Raises a
ValueError if any of the aforementioned conditions are False.

Parameters
----------
args: tuple
The `*args` parameter passed into a function

min_length: int, optional
The minimum number of arguments that should be contained in the `args`.
tuple. This number must be non-negative. The default is '0'.

max_length: int, optional
If not `None`, the maximum number of arguments that should be contained
in the `args` parameter. This number must be at least as large as the
provided `min_length` value. The default is None.

msg: str, optional
Error message to display when a custom check of args fails. For
example, pandas does not support a non-None argument for `out`
when rounding a `Series` or `DataFrame` object. `msg` in this
case can be "Inplace rounding is not supported".

Raises
------
ValueError if `args` fails to have a length that is at least `min_length`
and at most `max_length` inclusive (provided `max_length` is not None)

"""
length = len(args)

if min_length < 0:
raise ValueError("'min_length' must be non-negative")

if max_length is None:
if length < min_length:
raise ValueError(("expected at least {min_length} arguments "
"but got {length} arguments instead".
format(min_length=min_length, length=length)))

if min_length > max_length:
raise ValueError("'min_length' > 'max_length'")

if (length < min_length) or (length > max_length):
raise ValueError(("expected between {min_length} and {max_length} "
"arguments inclusive but got {length} arguments "
"instead".format(min_length=min_length,
length=length,
max_length=max_length)))

# See gh-12600; this is to allow compatibility with NumPy,
Copy link
Contributor

Choose a reason for hiding this comment

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

this is wrong. it should only be triggered if you <= max_length and >= min_length args, then it should check if any are not None.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. I think it's already being triggered when that's the case. Otherwise, some sort of ValueError is thrown.
  2. Are you saying that the check should be if all values in the tuple *args are None?

Copy link
Contributor

Choose a reason for hiding this comment

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

the ideas is that when we use this validator function it will allow extra args (according to min/max lens), but they all must be None, just a bit more general.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see. Done.

# which passes in an 'out' parameter as a positional argument
if args:
args = list(filter(lambda elt: elt is not None, args))

if args:
raise ValueError(msg)


def validate_kwargs(fname, kwargs, *compat_args):
"""
Checks whether parameters passed to the **kwargs argument in a
function 'fname' are valid parameters as specified in *compat_args

Parameters
----------
fname: str
The name of the function being passed the `**kwargs` parameter

kwargs: dict
The `**kwargs` parameter passed into `fname`

compat_args: *args
A tuple of keys that `kwargs` is allowed to have

Raises
------
ValueError if `kwargs` contains keys not in `compat_args`

"""
list(map(kwargs.__delitem__, filter(
kwargs.__contains__, compat_args)))
if kwargs:
bad_arg = list(kwargs)[0] # first 'key' element
raise TypeError(("{fname}() got an unexpected "
"keyword argument '{arg}'".
format(fname=fname, arg=bad_arg)))