-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
COMPAT: Further Expand Compatibility with fromnumeric.py #13148
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
COMPAT: Further Expand Compatibility with fromnumeric.py #13148
Conversation
@@ -70,6 +70,34 @@ New Behavior: | |||
|
|||
type(s.tolist()[0]) | |||
|
|||
numpy function compatibility | |||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just make this a 1-liner. This is not something we care to advertise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Done.
e5465a6
to
ca890e9
Compare
Current coverage is 84.14%@@ master #13148 diff @@
==========================================
Files 138 138
Lines 50394 50409 +15
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 42409 42416 +7
- Misses 7985 7993 +8
Partials 0 0
|
@gfyoung let's maybe just disallow using numpy functions on window/groupby ops (the this should just raise a nicer error message (say this is not supported)
|
Fair enough. So instead of validating as I do now, I just check if the |
85fa8da
to
8cb265d
Compare
not really sure how to figure out if called via numpy w/o stack inspection (not a good idea!). though you could simply just have it raise when it validates, e.g. its sort of duck like in the argument signature when called from numpy. IOW numpy is always passing dtype/out I think. So just raise a nicer error message on that (you prob need to pass a parameter to your validation routines to say hey if this is not validation, then print this nice error message; e.g. you want to say something like) """ numpy operations are not valid with groupy, use |
220272e
to
2a5817d
Compare
@@ -25,6 +25,14 @@ | |||
from pandas.compat import OrderedDict | |||
|
|||
|
|||
class BadNumpyCall(Exception): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm not sure I like the name. maybe UnsupportedFunctionCall
? and move to core.common. inherit from ValueError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works too. Will change.
659dd51
to
e92aa9f
Compare
@@ -46,7 +46,8 @@ API changes | |||
|
|||
|
|||
- Non-convertible dates in an excel date column will be returned without conversion and the column will be ``object`` dtype, rather than raising an exception (:issue:`10001`) | |||
|
|||
- Compatibility with ``fromnumeric.py`` in NumPy has been expanded to timestamps (:issue: `12811`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give a easier explanation? Users don't know what fromnumeric.py
is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Expands compatibility with fromnumeric.py in tslib.pyx and puts checks in window.py, groupby.py, and resample.py to ensure that pandas functions such as 'mean' are not called via the numpy library. Closes pandas-devgh-12811.
e92aa9f
to
eb4762c
Compare
@jreback, @jorisvandenbossche : Made the changes, and Travis is giving the green light. Ready to merge if there is nothing else. |
thanks @gfyoung |
@gfyoung breaking with numpy master on rounding: https://travis-ci.org/pydata/pandas/jobs/131735642 |
Okay, I think we might need to undo this compatibility change to # signature in tslib.pyx
def round(self, freq, *args, **kwargs):
...
# signature in numpy master
def around(a, decimals=0, out=None):
return _wrapfunc(a, 'round', decimals=decimals, out=out) When we call |
ok so just revert that part of the PR |
Title is self-explanatory. xref #13148. Author: gfyoung <[email protected]> Closes #13246 from gfyoung/tslib-compat-undo and squashes the following commits: 66160f9 [gfyoung] Reverse numpy compat changes to tslib.pyx
Follow-on to #12810 by expanding compatibility with
fromnumeric.py
in the following modules:tslib.pyx
window.py
groupby.py
andresample.py
(shared classes)Closes #12811.