Skip to content
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

error: Argument 1 to "mul" of "Series" has incompatible type "timedelta64" [arg-type] #748

Closed
randolf-scholz opened this issue Jul 18, 2023 · 4 comments · Fixed by #924
Closed
Labels
Numeric Operations Arithmetic, Comparison, and Logical operations

Comments

@randolf-scholz
Copy link
Contributor

randolf-scholz commented Jul 18, 2023

Describe the bug

Series.mul has incompatible signature with Series.__mul__

To Reproduce

import pandas as pd
import numpy as np

s = pd.Series([1, 2, 3])
x = s * np.timedelta64(1, "s")  # ✔
y = s.mul(np.timedelta64(1, "s") )  # ✘ [arg-type]

Please complete the following information:

  • OS: Ubuntu 22.04
  • python version 3.10
  • mypy 1.4.1
  • pandas 2.0.3
  • pandas-stubs 2.0.2.230605
@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jul 18, 2023

Need to coordinate the definitions of Series.mul() and Series.__mul__(), as well as tthe other operators (e.g., add() and __add__(), etc.

@Dr-Irv Dr-Irv added the Numeric Operations Arithmetic, Comparison, and Logical operations label Jul 18, 2023
mutricyl pushed a commit to mutricyl/pandas-stubs that referenced this issue May 14, 2024
@mutricyl
Copy link
Contributor

mutricyl commented May 14, 2024

Any kind of multiplication of a series with Timedeltas induce a TimedeltaSeries. I have updated mul, __mul__, rmul and __rmul__ to be coherent.

I have quickly tested + / // & | and they all raise an error when dealing with Series and Timedeltas. So I wonder if I should add a bunch of Never or keep as is managing only *. @Dr-Irv could you advise ?

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented May 14, 2024

I have quickly tested + / // & | and they all raise an error when dealing with Series and Timedeltas. So I wonder if I should add a bunch of Never or keep as is managing only *. @Dr-Irv could you advise ?

Let's leave the scope as is. The challenge here is that you could get a Series that contains timedeltas, but from a typing perspective, it is Series[Any], but addition is allowed at runtime.

So make a PR that just aligns the arguments for things like SOMEOP and __SOMEOP__ , where SOMEOP is mul, add, etc., with appropriate tests. I'm pretty sure that the arguments we use for the __SOMEOP__ versions are well tested so if you just copy those declarations over to SOMEOP and add tests for the SOMEOP versions, you'll be in good shape.

Separately, if you want to experiment with using Never to prevent operations, you could do a separate PR, and as long as none of the existing tests fail, it's something we could consider.

Thanks for your contributions!

@mutricyl
Copy link
Contributor

Series[int] or Series[float] works fine of course for * but fails for +

>>> s2 = pd.Series([1, 2, 3])                         
>>> s2 * np.timedelta64(1, "s")     
0   0 days 00:00:01
1   0 days 00:00:02
2   0 days 00:00:03
dtype: timedelta64[s]
>>> s2 + np.timedelta64(1, "s")
(...)
TypeError: Concatenation operation is not implemented for NumPy arrays, use np.concatenate() instead. Please do not rely on this error; it may not be given on all Python implementations.

however with Series[object] (as we mix int and timedelta):

>>> s = pd.Series([1, 2, 3, np.timedelta64(1, "s")]) 
>>> s
0            1
1            2
2            3
3    1 seconds
>>> s * np.timedelta64(1, "s")
(...)
numpy.core._exceptions._UFuncBinaryResolutionError: ufunc 'multiply' cannot use operands with types dtype('O') and dtype('<m8[s]')
>>> s + np.timedelta64(1, "s") 
(...)
TypeError: unsupported operand type(s) for +: 'Timedelta' and 'int'
>>> s[0:2] * np.timedelta64(1, "s")
(...)
numpy.core._exceptions._UFuncBinaryResolutionError: ufunc 'multiply' cannot use operands with types dtype('O') and dtype('<m8[s]')

+ and * are failling. (you can not multiply to timedeltas anyway)

I can not find any case where Series + Timedeltas are allowed. So I'll go for Never.

I will PR what I have done for * so far and give a try with Never all the other operators in a separate branch.

Dr-Irv pushed a commit that referenced this issue May 15, 2024
* adding tests for #748

* updating __rmul__, rmul and mul

---------

Co-authored-by: Laurent Mutricy <laurent.mutricy@ekium.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants