-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: do extract_array earlier in series arith/comparison ops #28066
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
Conversation
pandas/core/ops/__init__.py
Outdated
@@ -520,13 +524,29 @@ def column_op(a, b): | |||
return result | |||
|
|||
|
|||
def dispatch_to_extension_op(op, left, right): | |||
def dispatch_to_extension_op(op, left, right, keep_null_freq: bool = False): |
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.
@simonjayhawkins do we have a way of typing left
and right
as "not a Series or Index"?
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 u type left here (EA / np.ndarray)
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.
@simonjayhawkins do we have a way of typing
left
andright
as "not a Series or Index"?
i'm not aware of being able to exclude types.
if a particular type raises, (in this case TYpeError?) then maybe could use overloads with a return type of NoReturn https://mypy.readthedocs.io/en/latest/more_types.html#the-noreturn-type (New in version 3.5.4)
could maybe use the following pattern to allow checking with older Python...
if TYPE_CHECKING:
from typing import NoReturn
else:
NoReturn = None
pandas/core/ops/__init__.py
Outdated
@@ -520,13 +524,29 @@ def column_op(a, b): | |||
return result | |||
|
|||
|
|||
def dispatch_to_extension_op(op, left, right): | |||
def dispatch_to_extension_op(op, left, right, keep_null_freq: bool = False): |
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 u type left here (EA / np.ndarray)
Types added |
Can you expand on |
If we have a Series that is dt64, dt64tz, or td64, The exception is when we have a Series[int] and we're adding/subtracting a DTA/TDA/DTI/TDI, in which case NullFrequencyError is the correct thing to raise. |
So the complexity comes from having different rules for |
The DTA/TDA/DTI/TDI/Timestamp behavior has been deprecated, now just need to be patient |
So in the future |
sounds good |
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.
Since keep_null_freq
is just a temporary thing until the deprecation can be enforced, I'm +1 on this.
I'd recommend giving @jreback about 48 hours to look, and then merge if he hasn't had a chance by them.
thanks @jbrockmendel yeah the |
With this, the middle third of _arith_method_SERIES and _comp_method_SERIES are array-specific and can be refactored out (separate step) to become a) block-wise implementation for DataFrame and b) PandasArray implementation.
This also simplifies the NullFrequencyError handling nicely.