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

1109 athena datediff #1338

Merged
merged 35 commits into from
Jun 22, 2023
Merged

1109 athena datediff #1338

merged 35 commits into from
Jun 22, 2023

Conversation

RossKen
Copy link
Contributor

@RossKen RossKen commented Jun 15, 2023

Type of PR

  • BUG
  • FEAT
  • MAINT
  • DOC

Is your Pull Request linked to an existing Issue or Pull Request?

Closes #1300
Closes #1109

Give a brief description for the solution you have provided

Datediff is useful across linkers, athena has a native function for it, so we should include in our cll libraries

PR Checklist

  • Added documentation for changes
  • Added feature to example notebooks at tutorials in splink_demos (if appropriate)
  • Added tests (if appropriate)
  • Made changes based off the latest version of Splink
  • Run the linter

@github-actions
Copy link
Contributor

github-actions bot commented Jun 15, 2023

Test: test_2_rounds_1k_duckdb

Percentage change: -32.9%

date time stats_mean stats_min commit_info_branch commit_info_id machine_info_cpu_brand_raw machine_info_cpu_hz_actual_friendly commit_hash
849 2022-07-12 18:40:05 1.89098 1.87463 splink3 c334bb9 Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz 2.7934 GHz c334bb9
1749 2023-06-20 15:00:57 1.26525 1.2588 (detached head) 102faf7 Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz 2.5939 GHz 102faf7

Test: test_2_rounds_1k_sqlite

Percentage change: -19.4%

date time stats_mean stats_min commit_info_branch commit_info_id machine_info_cpu_brand_raw machine_info_cpu_hz_actual_friendly commit_hash
851 2022-07-12 18:40:05 4.32179 4.25898 splink3 c334bb9 Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz 2.7934 GHz c334bb9
1751 2023-06-20 15:00:57 3.43709 3.43356 (detached head) 102faf7 Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz 2.5939 GHz 102faf7

Click here for vega lite time series charts

@RossKen RossKen marked this pull request as ready for review June 20, 2023 15:00
col_name_l,
col_name_r,
date_threshold,
date_metric,
Copy link
Contributor

Choose a reason for hiding this comment

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

I take it that the user entering an unexpected date_metric (unexpected for us here being anything other than day, month or year), then it will still calculate that without any issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, if it is valid in presto it should accept it fine

Copy link
Contributor

@ThomasHepworth ThomasHepworth left a comment

Choose a reason for hiding this comment

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

I'm happy to approve this, even with the outstanding question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants