-
Notifications
You must be signed in to change notification settings - Fork 1.8k
add Atan2 #2942
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
add Atan2 #2942
Conversation
|
atan2(y, x) is implemented. Do we have any test cases for validating basic math function like sin, cos? I'd like to know what's the recommended way to deal with float number (might be something like assert_f64_near!?) |
Codecov Report
@@ Coverage Diff @@
## master #2942 +/- ##
==========================================
+ Coverage 85.41% 85.66% +0.24%
==========================================
Files 273 280 +7
Lines 49495 51144 +1649
==========================================
+ Hits 42278 43814 +1536
- Misses 7217 7330 +113
Help us with your feedback. Take ten seconds to tell us how you rate us. |
alamb
left a comment
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.
Thank you so much for the contribution @waitingkuo -- this looks great
| let sql = "SELECT atan2(y, x) FROM t1"; | ||
| let actual = execute_to_batches(&ctx, sql).await; | ||
|
|
||
| let expected = vec![ |
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.
alamb=# select atan2(2.0, 1.0);
atan2
--------------------
1.1071487177940904
(1 row)
alamb=# select atan2(-2.0, 1.0);
atan2
---------------------
-1.1071487177940904
(1 row)
alamb=# select atan2(2.0, -1.0);
atan2
--------------------
2.0344439357957027
(1 row)
alamb=# select atan2(-2.0, -1.0);
atan2
---------------------
-2.0344439357957027
(1 row)
👍
| } | ||
|
|
||
| pub fn atan2(args: &[ArrayRef]) -> Result<ArrayRef> { | ||
| // FIXME other data_type? |
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.
What does this comment mean? What other data types do you have in mind?
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.
(the type coercion already applies here)
For example, I can run this on integers for example in cargo datafusion-cli
❯ select atan2(2, 1);
+--------------------------+
| atan2(Int64(2),Int64(1)) |
+--------------------------+
| 1.1071488 |
+--------------------------+
❯ select atan2(cast(2 as int), cast(1 as int));
+--------------------------------------------------------+
| atan2(CAST(Int64(2) AS Int32),CAST(Int64(1) AS Int32)) |
+--------------------------------------------------------+
| 1.1071488 |
+--------------------------------------------------------+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 implemented f64 first and left this to remind me to implement f32. it turns out that i need another reminder to delete this comment...
fixed!
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.
if both args are f32, it will return f32 using https://doc.rust-lang.org/std/primitive.f32.html#method.atan2
|
Thanks @waitingkuo -! |
|
Benchmark runs are scheduled for baseline = cd31649 and contender = 176f432. 176f432 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2897
Rationale for this change
What changes are included in this PR?
atan2(y, x) expression
Are there any user-facing changes?