BUG: Matched Round Signature with NumPy's #12603
Conversation
gfyoung
force-pushed
the
round_out_compat
branch
from
b008963 to
b3f4634
Compare
jreback
added
the
Compat
|
@gfyoung when we deal with this issue. can you do a search on current numpy as see what functions |
|
Yep, sounds good to me as a follow-up PR. |
gfyoung
force-pushed
the
round_out_compat
branch
from
b3f4634 to
24bf53b
Compare
| @@ -4486,6 +4487,8 @@ def _series_round(s, decimals): | |||
| return s.round(decimals) | |||
| return s | |||
|
|
|||
| validate_args(args, min_length=0, max_length=1) | |||
|
|
|||
gfyoung
force-pushed
the
round_out_compat
branch
7 times, most recently
from
ad735d7 to
0280828
Compare
gfyoung
force-pushed
the
round_out_compat
branch
from
0280828 to
9242804
Compare
|
@jreback: Any updates on this PR? I've been running tests and diff-ing locally to make sure that Travis should be happy once I remove the |
| @@ -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): | |||
There was a problem hiding this comment.
are these passed by position or kw?
There was a problem hiding this comment.
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.
|
i c you have removed dependene on #12413 great |
gfyoung
force-pushed
the
round_out_compat
branch
2 times, most recently
from
8d6f8d0 to
6f88946
Compare
|
Added the validator for |
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
dont' touch my whitespace!! this is on purpose
gfyoung
force-pushed
the
round_out_compat
branch
2 times, most recently
from
7304b46 to
aad613b
Compare
| length=length, | ||
| max_length=max_length))) | ||
|
|
||
| # See gh-12600; this is to allow compatibility with NumPy, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
- I think it's already being triggered when that's the case. Otherwise, some sort of
ValueErroris thrown. - Are you saying that the check should be if all values in the tuple
*argsareNone?
There was a problem hiding this comment.
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.
gfyoung
force-pushed
the
round_out_compat
branch
from
aad613b to
ff9fc90
Compare
|
ok, looks good. ping when green (on your travis run is fine). |
|
@jreback : Travis has finally given the green light. Ready to merge if there's nothing else to change. |
|
ok, looks good. will merge in a bit. |
|
thanks @gfyoung for the patience! |
|
@jreback : No worries. Better to get it right the first time. 😄 |
Closes #12600 by re-accepting the
outparameter as a**kwargsargument. Also removes theoutparameter from the signature forDataFrame.roundand replaces it with**kwargslike itsSeriescounterpart. Furthermore, sinceoutis a parameter that is not supported inpandas, checks thatoutisNoneand throws an error if it isn't.