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

   **Is your feature request related to a problem or challenge? Please describe 
what you are trying to do.**
   
   When adding additional support to the statistics converter as @Kev1n8 did in 
https://github.com/apache/arrow-rs/pull/6181 the presence of two sets of tests 
is quite confusing about where to add tests for the new test
   
   
   There are tests in both
   * 
https://github.com/apache/arrow-rs/blob/master/parquet/src/arrow/arrow_reader/statistics.rs
   * 
https://github.com/apache/arrow-rs/blob/master/parquet/tests/arrow_reader/statistics.rs
   
   
   
   **Describe the solution you'd like**
   I would like to remove the rundant tests 
   
   **Describe alternatives you've considered**
   I think the clearest alternateive is to move all the tests from 
https://github.com/apache/arrow-rs/blob/master/parquet/src/arrow/arrow_reader/statistics.rs
 to  
https://github.com/apache/arrow-rs/blob/master/parquet/tests/arrow_reader/statistics.rs
   
   We should only move the non duplicated tests.
   
   So that means something like:
   - [ ] Leave a comment with a note about where the tests are in 
https://github.com/apache/arrow-rs/blob/master/parquet/src/arrow/arrow_reader/statistics.rs
   - [ ] Keep edge condition tests 
https://github.com/apache/arrow-rs/blob/f2de2cd3c8a8496e1f96e58b50786227bfe915f7/parquet/src/arrow/arrow_reader/statistics.rs#L1513-L1522
   - [ ] Remove tests in 
https://github.com/apache/arrow-rs/blob/f2de2cd3c8a8496e1f96e58b50786227bfe915f7/parquet/src/arrow/arrow_reader/statistics.rs#L1548-L2113
 that have corresponding coverage
   - [ ] Keep data file validation: 
https://github.com/apache/arrow-rs/blob/f2de2cd3c8a8496e1f96e58b50786227bfe915f7/parquet/src/arrow/arrow_reader/statistics.rs#L2115-L2235
   - [ ] Port fixed length decimal legacy: 
https://github.com/apache/arrow-rs/blob/f2de2cd3c8a8496e1f96e58b50786227bfe915f7/parquet/src/arrow/arrow_reader/statistics.rs#L2237-L2258
   
   
   
   
   **Additional context**
   This duplication is a historical artifact of how this code was developed in 
DataFusion and then it got brought over when @efredine  ported the work in 
https://github.com/apache/arrow-rs/pull/6046


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to