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

feat(rust, python, cli): Adds (Most) Remaining Trig Functions to SQLContext #9453

Merged
merged 8 commits into from
Jun 21, 2023
Merged

feat(rust, python, cli): Adds (Most) Remaining Trig Functions to SQLContext #9453

merged 8 commits into from
Jun 21, 2023

Conversation

SeanTroyUWO
Copy link
Contributor

Allows the following to be used in SQL with SQLContext

  • COS
  • COT
  • SIN
  • TAN
  • COSD
  • COTD
  • SIND
  • TAND
  • ACOSD
  • ASIND
  • ATAND

Some functions were already usable, but apparently not documented

  • ASIN
  • ACOS
  • ATAN

I'm getting change suggestions from: #7227

@stinodego stinodego changed the title Adds (Most) Remaining Trig Functions to SQLContext feat(rust, python): Adds (Most) Remaining Trig Functions to SQLContext Jun 20, 2023
@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars labels Jun 20, 2023
@stinodego
Copy link
Contributor

Can you please rebase this so that the CI checks can run?

@SeanTroyUWO SeanTroyUWO force-pushed the RemainingTrigFunctions branch from 40811fb to 136a0bb Compare June 20, 2023 04:35
@SeanTroyUWO
Copy link
Contributor Author

Can you please rebase this so that the CI checks can run?

I think I've rebased the wrong direction... or something... I'm going to try to clean this up....

@SeanTroyUWO SeanTroyUWO force-pushed the RemainingTrigFunctions branch from 136a0bb to a1d6fc0 Compare June 20, 2023 05:24
@alexander-beedie alexander-beedie added the A-sql Area: Polars SQL functionality label Jun 20, 2023
@SeanTroyUWO SeanTroyUWO force-pushed the RemainingTrigFunctions branch from a1d6fc0 to 36c0125 Compare June 20, 2023 05:43
ritchie46
ritchie46 reviewed Jun 20, 2023
View reviewed changes
Copy link
Member

@ritchie46 ritchie46 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @SeanTroyUWO. If the new functions are exposed to python we can merge this.

polars/polars-lazy/polars-plan/src/dsl/arithmetic.rs
/// Compute the inverse hyperbolic tangent of the given expression
#[cfg(feature = "trigonometry")]
pub fn arctanh(self) -> Self {
self.map_private(FunctionExpr::Trigonometry(TrigonometricFunction::ArcTanh))
}

/// Convert from radians to degrees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also expose the new functions to the python API?

They should also get an entry in the python docs.

Copy link
Contributor Author

@SeanTroyUWO SeanTroyUWO Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added them to general.rs, please let me know if that looks good. I can add unit tests from the different entry point aswell if requested.

Edit: Added to the python API and added to the python docs. From local build:
image

universalmind303
universalmind303 requested changes Jun 20, 2023
View reviewed changes
polars/polars-sql/src/functions.rs Show resolved Hide resolved
polars/polars-sql/src/functions.rs Outdated Show resolved Hide resolved
@universalmind303 universalmind303 changed the title feat(rust, python): Adds (Most) Remaining Trig Functions to SQLContext feat(rust, python, cli): Adds (Most) Remaining Trig Functions to SQLContext Jun 20, 2023
SeanTroyUWO
SeanTroyUWO commented Jun 20, 2023
View reviewed changes
py-polars/tests/unit/test_sql.py

df_result = pl.DataFrame(
{
"pi values": [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make pre-commit makes this super long formatting, not sure if anyone has a good remedy for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave it, as long as it is automatic. :)

@ritchie46
Copy link
Member

Thanks a lot @SeanTroyUWO!

@ritchie46 ritchie46 merged commit 1c7744d into pola-rs:main Jun 21, 2023
@SeanTroyUWO SeanTroyUWO mentioned this pull request Jun 26, 2023
Merged
@SeanTroyUWO
Copy link
Contributor Author

SeanTroyUWO commented Jun 26, 2023 •
edited
Loading

@ritchie46 @universalmind303 I think I might have missed something. I'm not getting Expr.radians and Expr.degrees to show up in my Vscode IntelliSense
image
(Notice the colors are different)
Nevermind, ignore this...

c-peters pushed a commit to c-peters/polars that referenced this pull request Jul 14, 2023
@SeanTroyUWO @c-peters
feat(rust, python, cli): Adds (Most) Remaining Trig Functions to `SQL…
1d4156c 
…Context` (pola-rs#9453)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ritchie46 ritchie46 ritchie46 left review comments

@universalmind303 universalmind303 universalmind303 requested changes

@stinodego stinodego Awaiting requested review from stinodego

@alexander-beedie alexander-beedie Awaiting requested review from alexander-beedie alexander-beedie is a code owner

@MarcoGorelli MarcoGorelli Awaiting requested review from MarcoGorelli MarcoGorelli is a code owner

Assignees
No one assigned
Labels
A-sql Area: Polars SQL functionality enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SeanTroyUWO @stinodego @ritchie46 @universalmind303 @alexander-beedie