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

Add timezone support to JSON reader #3845

Merged
merged 3 commits into from
Mar 17, 2023

Conversation

tustvold
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label Mar 11, 2023
@@ -287,30 +287,28 @@ impl ArrowTemporalType for DurationMicrosecondType {}
impl ArrowTemporalType for DurationNanosecondType {}

/// A timestamp type allows us to create array builders that take a timestamp.
pub trait ArrowTimestampType: ArrowTemporalType {
pub trait ArrowTimestampType: ArrowTemporalType<Native = i64> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a breaking change as these types are sealed, but it simplifies generic code as i64 is guaranteed

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand how these are sealed -- https://docs.rs/arrow/35.0.0/arrow/datatypes/trait.ArrowTimestampType.html

Maybe we should add a doc commen note here explaining that they are sealed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I assumed they were, they definitely should be 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -287,30 +287,28 @@ impl ArrowTemporalType for DurationMicrosecondType {}
impl ArrowTemporalType for DurationNanosecondType {}

/// A timestamp type allows us to create array builders that take a timestamp.
pub trait ArrowTimestampType: ArrowTemporalType {
pub trait ArrowTimestampType: ArrowTemporalType<Native = i64> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand how these are sealed -- https://docs.rs/arrow/35.0.0/arrow/datatypes/trait.ArrowTimestampType.html

Maybe we should add a doc commen note here explaining that they are sealed

@tustvold tustvold merged commit eacb135 into apache:master Mar 17, 2023
spebern pushed a commit to spebern/arrow-rs that referenced this pull request Mar 25, 2023
* Add timezone support to JSON reader

* Fix doc
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants