You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/27 10:38:41 UTC

[GitHub] [arrow-rs] alamb opened a new issue #230: Add support for pretty-printing Decimal numbers

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


   In the context of https://github.com/apache/arrow-datafusion/issues/107 from @joshuataylor 
   
   **Describe the solution you'd like**
   It should be possible to pretty-print columns of `DataType::Decimal` type. Pretty printing is used for datafusion and other projects to make human readable versions of Arrow arrays.
   
   To do so, we would add support here:  https://github.com/apache/arrow-rs/blob/cf0c7d2cbecc2ca6ceeb7459417cef3501f64ad1/arrow/src/util/display.rs#L269-L271
   
   Right now, you get an error such as 
   ```
   ArrowError(InvalidArgumentError("Pretty printing not implemented for Decimal(9, 0) 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.

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



[GitHub] [arrow-rs] mgill25 edited a comment on issue #230: Add support for pretty-printing Decimal numbers

Posted by GitBox <gi...@apache.org>.
mgill25 edited a comment on issue #230:
URL: https://github.com/apache/arrow-rs/issues/230#issuecomment-830857889


   Hi @alamb I'd like to take this issue up if that's ok.
   
   Looks like the change should be simple enough - just add the `Decimal` pattern to the  match expression. I have a silly question however: Is this being tested somewhere?
   
   Seems like there is no test in `display.rs`, but there _are_ tests in `pretty.rs`. Which I tried to run these tests, it seems cargo is skipping them, and yet it doesn't skip the tests in other neighboring modules.
   
   <img width="1257" alt="Screenshot 2021-05-02 at 21 14 50" src="https://user-images.githubusercontent.com/1311924/116824681-c8a51d00-ab8b-11eb-8f33-03a6cfafe8c9.png">
   
   Am I overthinking this a bit? Let me know how to figure this one out. :) Thanks!
   
   Reason for tests: I'd like to see the change _in action_, so to speak.
   


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

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



[GitHub] [arrow-rs] mgill25 commented on issue #230: Add support for pretty-printing Decimal numbers

Posted by GitBox <gi...@apache.org>.
mgill25 commented on issue #230:
URL: https://github.com/apache/arrow-rs/issues/230#issuecomment-830857889


   Hi @alamb I'd like to take this issue up if that's ok.
   
   Looks like the change should be simple enough - just add the `Decimal` pattern to the  match expression. I have a silly question however: Is this being tested somewhere?
   
   Seems like there is no test in `display.rs`, but there _are_ tests in `pretty.rs`. Which I tried to run these tests, it seems cargo is skipping them, and yet it doesn't skip the tests in other neighboring modules.
   
   <img width="1257" alt="Screenshot 2021-05-02 at 21 14 50" src="https://user-images.githubusercontent.com/1311924/116824681-c8a51d00-ab8b-11eb-8f33-03a6cfafe8c9.png">
   
   Am I overthinking this a bit? Let me know how to figure this one out. :) Thanks!
   


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

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



[GitHub] [arrow-rs] nevi-me commented on issue #230: Add support for pretty-printing Decimal numbers

Posted by GitBox <gi...@apache.org>.
nevi-me commented on issue #230:
URL: https://github.com/apache/arrow-rs/issues/230#issuecomment-830878616


   @mgill25 the `pretty` tests are likely skipped because `prettyprint`is an optional feature


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

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



[GitHub] [arrow-rs] alamb closed issue #230: Add support for pretty-printing Decimal numbers

Posted by GitBox <gi...@apache.org>.
alamb closed issue #230:
URL: https://github.com/apache/arrow-rs/issues/230


   


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

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



[GitHub] [arrow-rs] alamb commented on issue #230: Add support for pretty-printing Decimal numbers

Posted by GitBox <gi...@apache.org>.
alamb commented on issue #230:
URL: https://github.com/apache/arrow-rs/issues/230#issuecomment-831199238


   Thanks @mgill25 ! As @nevi-me  says the pretty print feature is behind a feature flag (as it baloons the dependency stack with something that does not support for WASM builds, as I recall)
   
   To answer your question, the pretty print feature is tested here in CI:
   https://github.com/apache/arrow-rs/blob/master/.github/workflows/rust.yml#L114
   
   To test it locally, you can run `cargo` like:
   ```
             cargo test --features=prettyprint
   ```


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

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