You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "codephage2020 (via GitHub)" <gi...@apache.org> on 2026/02/28 02:04:50 UTC

[I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

codephage2020 opened a new issue, #9490:
URL: https://github.com/apache/arrow-rs/issues/9490

   **Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
   <!--
   A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] 
   (This section helps Arrow developers understand the context and *why* for this feature, in addition to  the *what*)
   -->
   
   Background: In PR #9345 , we added from_datetime and from_naive_datetime methods to the ArrowTimestampType trait. The from_naive_datetime(naive, None) call is now functionally equivalent to make_value(naive).
   
   **Describe the solution you'd like**
   <!--
   A clear and concise description of what you want to happen.
   -->
   Deprecate the make_value method on ArrowTimestampType trait 
   
   Migration Path: Users should replace:
   
   ```rust
   // Old
   TimestampSecondType::make_value(naive)
   ```
   With:
   
   ```rust
   // New
   TimestampSecondType::from_naive_datetime(naive, None)
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

Posted by "brunal (via GitHub)" <gi...@apache.org>.
brunal commented on issue #9490:
URL: https://github.com/apache/arrow-rs/issues/9490#issuecomment-3977004005

   I did not realize `make_value` was simply doing `and_utc`. Looking at the spec ([C++ DataType](https://arrow.apache.org/docs/cpp/api/datatype.html#_CPPv4N5arrow13TimestampTypeE), [Rust Arrow IPC](https://arrow.apache.org/rust/arrow_ipc/gen/Schema/struct.Timestamp.html)), this all makes sense. Keeping `from_naive_datetime(naive_dt)` is sensible, even if the user could call `and_utc()` themselves, as that matches the semantics of a timestamp array with unset/empty timezone.
   
   One thing that bothers me is that there's no link between `from_datetime`/`from_naive_datetime` usage & the timestamp set for the array in the schema. i.e. one can have a `DataType::Timestamp(TimeUnit::Millisecond, None)` field and build the data with `from_datetime`, losing the timezone guarantee, or (worse) have a `DataType::Timestamp(TimeUnit::Millisecond, Some("+08:00"))` field and build the data with `from_naive_datetime`. They provided something naive, but they end up with a timezone contract. Unfortunately, I don't think that can trivially be solved.
   
   Sorry for the rant. The only take-away is that I  confirm I believe the `tz` arg from `from_naive_datetime(dt, tz)` to be a mistake.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9490:
URL: https://github.com/apache/arrow-rs/issues/9490#issuecomment-4084935891

   `label_issue.py` automatically added labels {'parquet-variant'} from #9491


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

Posted by "codephage2020 (via GitHub)" <gi...@apache.org>.
codephage2020 commented on issue #9490:
URL: https://github.com/apache/arrow-rs/issues/9490#issuecomment-3976677823

   Your idea mostly makes sense to me. The design of this API originates from (here)[https://github.com/apache/arrow-rs/issues/9337#issuecomment-3842152008].
   If left to the user's discretion, the API would indeed be more concise and orthogonal.
   It would look like this:
   ```rust
   pub trait ArrowTimestampType: ArrowTemporalType<Native = i64> {
       const UNIT: TimeUnit;
   /// Create from a naive local time (interpreted as UTC)
   fn from_naive_datetime(naive: NaiveDateTime) -> Option<i64> {
       Self::from_datetime(naive.and_utc())
   }
   
   /// Create from a DateTime with timezone
   fn from_datetime<Tz: TimeZone>(datetime: DateTime<Tz>) -> Option<i64>;
   
   // Remove from_naive_datetime(naive, Option<&Tz>)
   
   }
   ```
   What are your thoughts? @alamb @sdf-jkl 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9490:
URL: https://github.com/apache/arrow-rs/issues/9490#issuecomment-4084935575

   `label_issue.py` automatically added labels {'arrow'} from #9491


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

Posted by "brunal (via GitHub)" <gi...@apache.org>.
brunal commented on issue #9490:
URL: https://github.com/apache/arrow-rs/issues/9490#issuecomment-3976616021

   Sorry I'm a bit late to the party -- why does `from_naive_datetime` accept a timezone? As that makes it a proper datetime. What's the point of accepting `from_naive_datetime(dt, Some(tz))` instead of making the user call `tz.from_local_datetime(&dt)` + handle ambiguosity themselves? Then you'd get proper orthogonal API endpoints.
   
   Right now the sensible case for `from_naive_datetime` is NOT to pass a tz, and one ends up passing an unclear `None` arg.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime [arrow-rs]

Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich closed issue #9490: Deprecate ArrowTimestampType::make_value in favor of from_naive_datetime
URL: https://github.com/apache/arrow-rs/issues/9490


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org