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

[PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Monkwire3 opened a new pull request, #5428:
URL: https://github.com/apache/arrow-rs/pull/5428

   # Which issue does this PR close?
   
   Closes #5336 .
   
   # Rationale for this change
    This change makes the debug output of Time32/Time64 arrays more descriptive.
   
   # What changes are included in this PR?
   This PR changes the way Time32 and Time64 arrays are printed to debug. Previously, values that could not be cast to Time32 or Time64 had debug output values of `null`. With this change, unsuccessful casts are included as errors in the debug output.
   
   # Are there any user-facing changes?
   Yes, the debug output for Time32 and Time64 arrays will change.
   


-- 
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: [PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Posted by "Monkwire3 (via GitHub)" <gi...@apache.org>.
Monkwire3 commented on PR #5428:
URL: https://github.com/apache/arrow-rs/pull/5428#issuecomment-1965547282

   Thank you!


-- 
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: [PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #5428:
URL: https://github.com/apache/arrow-rs/pull/5428


-- 
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: [PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Posted by "Monkwire3 (via GitHub)" <gi...@apache.org>.
Monkwire3 closed pull request #5428: Improve debug output of Time32/Time64 arrays
URL: https://github.com/apache/arrow-rs/pull/5428


-- 
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: [PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5428:
URL: https://github.com/apache/arrow-rs/pull/5428#issuecomment-1965166334

   I took the liberty of fixing the compilation errors in order to get this in


-- 
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: [PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5428:
URL: https://github.com/apache/arrow-rs/pull/5428#discussion_r1502043575


##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -1093,20 +1093,33 @@ where
 impl<T: ArrowPrimitiveType> std::fmt::Debug for PrimitiveArray<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         let data_type = self.data_type();
+
         write!(f, "PrimitiveArray<{data_type:?}>\n[\n")?;
         print_long_array(self, f, |array, index, f| match data_type {
             DataType::Date32 | DataType::Date64 => {
                 let v = self.value(index).to_isize().unwrap() as i64;
                 match as_date::<T>(v) {
                     Some(date) => write!(f, "{date:?}"),
-                    None => write!(f, "null"),
+                    None => {
+                        let err = ArrowError::CastError(format!(
+                            "Failed to convert {} to temporal for {:?}",
+                            v, data_type
+                        ));
+                        write!(f, "{}", err)

Review Comment:
   ```suggestion
                           write!(
                               "Cast error: Failed to convert {} to temporal for {:?}",
                               v, data_type
                           );
   ```



##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -1093,20 +1093,33 @@ where
 impl<T: ArrowPrimitiveType> std::fmt::Debug for PrimitiveArray<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         let data_type = self.data_type();
+
         write!(f, "PrimitiveArray<{data_type:?}>\n[\n")?;
         print_long_array(self, f, |array, index, f| match data_type {
             DataType::Date32 | DataType::Date64 => {
                 let v = self.value(index).to_isize().unwrap() as i64;
                 match as_date::<T>(v) {
                     Some(date) => write!(f, "{date:?}"),
-                    None => write!(f, "null"),
+                    None => {
+                        let err = ArrowError::CastError(format!(
+                            "Failed to convert {} to temporal for {:?}",
+                            v, data_type
+                        ));
+                        write!(f, "{}", err)
+                    }
                 }
             }
             DataType::Time32(_) | DataType::Time64(_) => {
                 let v = self.value(index).to_isize().unwrap() as i64;
                 match as_time::<T>(v) {
                     Some(time) => write!(f, "{time:?}"),
-                    None => write!(f, "null"),
+                    None => {
+                        let err = ArrowError::CastError(format!(
+                            "Failed to convert {} to temporal for {:?}",
+                            v, data_type
+                        ));
+                        write!(f, "{}", err)

Review Comment:
   ```suggestion
                           write!(
                               "Cast error: Failed to convert {} to temporal for {:?}",
                               v, data_type
                           );
   ```



-- 
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: [PR] Improve debug output of Time32/Time64 arrays [arrow-rs]

Posted by "Monkwire3 (via GitHub)" <gi...@apache.org>.
Monkwire3 commented on PR #5428:
URL: https://github.com/apache/arrow-rs/pull/5428#issuecomment-1963323087

   Thanks, you are right, that looks better. 


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