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

Update latest chrono #5479

Merged
merged 1 commit into from
Mar 7, 2024
Merged

Conversation

tustvold
Copy link
Contributor

@tustvold tustvold commented Mar 6, 2024

Which issue does this PR close?

Closes #.

Rationale for this change

New chrono, new deprecations.

This just does the bare minimum to get the CI green, follow on work is tracked in #4456 and #5480

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added arrow Changes to the arrow crate object-store Object Store Interface labels Mar 6, 2024
@@ -1001,7 +1004,7 @@ impl Date32Type {
/// * `i` - The Date32Type to convert
pub fn to_naive_date(i: <Date32Type as ArrowPrimitiveType>::Native) -> NaiveDate {
let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
epoch.add(Duration::days(i as i64))
epoch.add(Duration::try_days(i as i64).unwrap())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making these not panic is tracked by #4456

@@ -130,45 +131,48 @@ pub fn time_to_time64ns(v: NaiveTime) -> i64 {
/// converts a `i64` representing a `timestamp(s)` to [`NaiveDateTime`]
#[inline]
pub fn timestamp_s_to_datetime(v: i64) -> Option<NaiveDateTime> {
NaiveDateTime::from_timestamp_opt(v, 0)
Some(DateTime::from_timestamp(v, 0)?.naive_utc())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#5480 tracks cleaning up these APIs to return DateTime<Utc>

@tustvold
Copy link
Contributor Author

tustvold commented Mar 7, 2024

Merging this as the changes are mechanical and this is blocking the CI pipelines

@tustvold tustvold merged commit ae1d10f into apache:master Mar 7, 2024
32 checks passed
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @tustvold -- I took a look at this PR and the changes make sense to me

@@ -179,13 +183,13 @@ pub(crate) fn split_second(v: i64, base: i64) -> (i64, u32) {
/// converts a `i64` representing a `duration(s)` to [`Duration`]
#[inline]
pub fn duration_s_to_duration(v: i64) -> Duration {
Duration::seconds(v)
Duration::try_seconds(v).unwrap()
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any performance implication here (e.g. starting to check for invalid durations when we didn't before)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it just moves the panic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate object-store Object Store Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants