[GitHub] [arrow-datafusion] retikulum commented on pull request #4004: Improve Error Handling and Readibility for downcasting `Date32Array`

[GitBox <gi...@apache.org>]

retikulum commented on PR #4004:
URL: https://github.com/apache/arrow-datafusion/pull/4004#issuecomment-1295817035

   > Thanks @retikulum -- looks neat. I left some suggestions but basically this is the right track I think
   > 
   > what do you think @avantgardnerio ?
   
   Thanks for great feedback @alamb . Your recommendations generally improve my style to write Rust. I will make necessary changes.
   
   
   > > what do you think @avantgardnerio ?
   > 
   > I 100% agree with the intent of the PR: even if they "never happen at run time" panics (and unwraps, etc) create cognitive load on the programmer to mentally statically prove that to themselves every time they look at the code, so we should just eliminate them to the extent possible.
   > 
   > As far as implementation, as I become more proficient at Rust, I increasingly see the importance of like this being discoverable... is it possible to make this change in `arrow::ArrayRef`? Or to do a `Date32::TryFrom` trait? In the sea of code I wade through every day, anything that autocompletes is a lot more likely to get used.
   
   Hi @avantgardnerio. Thanks for the feedback and for joining the conversation. As far as I understand, We are on right track but we need to decide how to implement those changes. Also, this is discussed in #3152. @alamb what do you think about implementation? Should we continue creating functions in `datafusion\common\src\cast.rs` or create a different schema? If we will follow a different path, could you also help me out with how it is implemented?


-- 
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