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.

Sign up for GitHub

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

Jump to bottom

[C++][Python] Add division kernel: duration / duration -> float #36789

Closed
randolf-scholz opened this issue Jul 20, 2023 · 4 comments · Fixed by #36800
Closed

[C++][Python] Add division kernel: duration / duration -> float #36789

randolf-scholz opened this issue Jul 20, 2023 · 4 comments · Fixed by #36800
Assignees
@js8544
Labels
Component: C++ Component: Python Type: enhancement
Milestone
14.0.0

Comments

@randolf-scholz
Copy link

Describe the enhancement requested

Numpy allows division of timedelta64 by timedelta64 types, resulting in plain floating points (units cancel!)

import pyarrow as pa
import numpy as np

one_second_numpy = np.timedelta64(1, "s")
one_second_arrow = pa.scalar(1, pa.duration("s"))

assert one_second_numpy / one_second_numpy == 1.0  # ✔
assert one_second_arrow / one_second_arrow == 1.0  # TypeError: unsupported operand

Component(s)

Python
@randolf-scholz randolf-scholz changed the title Add division kernel: duration / duration -> float [C++][Python] Add division kernel: duration / duration -> float Jul 20, 2023
@js8544
Copy link
Collaborator

js8544 commented Jul 21, 2023

I'll add a kernel today.

@js8544 js8544 self-assigned this Jul 21, 2023
@github-actions github-actions bot mentioned this issue Jul 21, 2023
Merged
pitrou pushed a commit that referenced this issue Jul 25, 2023
@js8544
GH-36789: [C++] Support divide(duration, duration) (#36800)
dbae5f0 
### Rationale for this change

Support divide(duration, duration), as pandas and numpy already do.

### What changes are included in this PR?

Add kernels divide(duration, duration)->float64 and divide_checked(duration, duration)->float64

### Are these changes tested?

Yes

### Are there any user-facing changes?

No

* Closes: #36789

Authored-by: Jin Shang <shangjin1997@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 14.0.0 milestone Jul 25, 2023
R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
@js8544 @R-JunmingChen
apacheGH-36789: [C++] Support divide(duration, duration) (apache#36800)
ae84509 
### Rationale for this change

Support divide(duration, duration), as pandas and numpy already do.

### What changes are included in this PR?

Add kernels divide(duration, duration)->float64 and divide_checked(duration, duration)->float64

### Are these changes tested?

Yes

### Are there any user-facing changes?

No

* Closes: apache#36789

Authored-by: Jin Shang <shangjin1997@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@js8544 @loicalleyne
apacheGH-36789: [C++] Support divide(duration, duration) (apache#36800)
bec07d5 
### Rationale for this change

Support divide(duration, duration), as pandas and numpy already do.

### What changes are included in this PR?

Add kernels divide(duration, duration)->float64 and divide_checked(duration, duration)->float64

### Are these changes tested?

Yes

### Are there any user-facing changes?

No

* Closes: apache#36789

Authored-by: Jin Shang <shangjin1997@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@randolf-scholz randolf-scholz mentioned this issue Dec 9, 2023
Closed
@randolf-scholz
Copy link
Author

@js8544 It seems that the current kernel implementation is broken for negative durations: #39156

@js8544
Copy link
Collaborator

js8544 commented Dec 9, 2023

I confirm I can reproduce this bug. Let's me see how to fix it.

@js8544
Copy link
Collaborator

js8544 commented Dec 9, 2023

I've submitted a fix: #39158

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@js8544 js8544

Labels
Component: C++ Component: Python Type: enhancement
Projects
None yet
Milestone
14.0.0
Development

Successfully merging a pull request may close this issue.

GH-36789: [C++] Support divide(duration, duration) js8544/arrow
3 participants
@pitrou @js8544 @randolf-scholz