perf: Make .dt.weekday 20x faster
#17992
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcoGorelli
commented
Aug 1, 2024
Comment on lines
+2287
to
+2311
| @given( | ||
| value=st.datetimes(min_value=datetime(1800, 1, 1), max_value=datetime(2100, 1, 1)), | ||
| time_zone=st.sampled_from(["UTC", "Asia/Kathmandu", "Europe/Amsterdam", None]), | ||
| time_unit=st.sampled_from(["ms", "us", "ns"]), | ||
| ) | ||
| def test_weekday_vs_stdlib_datetime( | ||
| value: datetime, time_zone: str, time_unit: TimeUnit | ||
| ) -> None: | ||
| result = ( | ||
| pl.Series([value], dtype=pl.Datetime(time_unit)) | ||
| .dt.replace_time_zone(time_zone) | ||
| .dt.weekday() | ||
| .item() | ||
| ) | ||
| expected = value.isoweekday() | ||
| assert result == expected | ||
|
|
||
|
|
||
| @given( | ||
| value=st.dates(), | ||
| ) | ||
| def test_weekday_vs_stdlib_date(value: date) -> None: | ||
| result = pl.Series([value]).dt.weekday().item() | ||
| expected = value.isoweekday() | ||
| assert result == expected |
There was a problem hiding this comment.
Hypothesis-testing the hell out of this as you can't be too careful here 😇
MarcoGorelli
changed the title
.dt.weekday 20x faster.dt.weekday 20x faster
github-actions
bot
added
performance
Awesome. Have I already mentioned chrono is slow? :D |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17992 +/- ##
==========================================
+ Coverage 80.40% 80.49% +0.08%
==========================================
Files 1494 1496 +2
Lines 196480 196646 +166
Branches 2817 2817
==========================================
+ Hits 157982 158292 +310
+ Misses 37978 37833 -145
- Partials 520 521 +1 ☔ View full report in Codecov by Sentry. |
MarcoGorelli
requested review from
ritchie46,
c-peters,
alexander-beedie,
reswqa and
orlp
as code owners
ritchie46
approved these changes
Aug 1, 2024
wence-
reviewed
Aug 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR: use maths instead of Chrono