feat: convert and replace timezone#6106
Open
aaron-ang wants to merge 1 commit intoEventual-Inc:mainfrom
Open
Conversation
Contributor
Greptile OverviewGreptile SummaryThis PR implements timezone conversion and replacement functionality for timestamp types. The implementation introduces a new Key Changes:
Test Coverage:
The implementation follows Daft's new array building semantics and properly validates inputs at both the type-checking and runtime levels. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Python API
participant Series
participant UDF
participant Array
participant TimeUnit
User->>Python API: convert_time_zone(expr, to_tz, from_tz?)
Python API->>Series: dt_convert_time_zone(to_tz, from_tz?)
Series->>UDF: ConvertTimeZone::call(...)
UDF->>Array: convert_time_zone(to_tz, from_tz?)
Array->>TimeUnit: parse_timezone(to_tz)
TimeUnit-->>Array: ParsedTimezone
alt timestamp has timezone
Array->>Array: Return with updated timezone metadata
else timestamp without timezone
alt from_timezone provided
Array->>TimeUnit: parse_timezone(from_tz)
TimeUnit-->>Array: ParsedTimezone
Array->>TimeUnit: naive_local_to_timestamp(...)
TimeUnit-->>Array: converted timestamp
else from_timezone missing
Array-->>User: Error: from_timezone required
end
end
Array-->>UDF: TimestampArray
UDF-->>Series: Series
Series-->>User: Result
User->>Python API: replace_time_zone(expr, tz?)
Python API->>Series: dt_replace_time_zone(tz?)
Series->>UDF: ReplaceTimeZone::call(...)
UDF->>Array: replace_time_zone(tz?)
Array->>TimeUnit: timestamp_to_naive_local(...)
TimeUnit-->>Array: naive datetime
alt new timezone provided
Array->>TimeUnit: naive_local_to_timestamp(...)
TimeUnit-->>Array: new timestamp
else timezone = None
Array->>TimeUnit: naive_datetime_to_timestamp(...)
TimeUnit-->>Array: timestamp as UTC
end
Array-->>UDF: TimestampArray
UDF-->>Series: Series
Series-->>User: Result
Last reviewed commit: 75d6e27 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Author
|
@greptile-apps re-review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6106 +/- ##
==========================================
- Coverage 43.34% 43.12% -0.23%
==========================================
Files 917 920 +3
Lines 112995 113929 +934
==========================================
+ Hits 48978 49131 +153
- Misses 64017 64798 +781
🚀 New features to boost your workflow:
|
aaron-ang
force-pushed
the
tz-convert
branch
2 times, most recently
from
8863882 to
a735dce
Compare
Contributor
Author
|
@greptile-apps re-review |
src/daft-core/src/array/ops/cast.rs
Outdated
| .unwrap_or_else(|_| panic!("Unable to parse timezone string {tz}")) | ||
| }); | ||
| let str_array: daft_arrow::array::Utf8Array<i64> = self | ||
| .as_arrow2() |
Member
There was a problem hiding this comment.
nit: We'd prefer to not introduce any new arrow2 based code. Could you update this to use arrow-rs instead?
aaron-ang
force-pushed
the
tz-convert
branch
from
bff0bd1 to
0aad97b
Compare
aaron-ang
force-pushed
the
tz-convert
branch
from
c40ba3a to
75d6e27
Compare
Contributor
Author
|
rebased |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Changes Made
from_timezoneis not provided for timestamp without tz info.from_timezoneif timestamp already has tz info.timezoneis not provided.ParsedTimezoneenum to handle different timezone types (offset and tz name).Related Issues
Closes #4096.