[GitHub] [arrow-rs] alamb commented on a change in pull request #731: fix: Allow parquet to be compiled without arrow (fix --no-default-features)

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

alamb commented on a change in pull request #731:
URL: https://github.com/apache/arrow-rs/pull/731#discussion_r699231797



##########
File path: parquet/src/util/bit_util.rs
##########
@@ -680,6 +680,15 @@ impl From<Vec<u8>> for BitReader {
     }
 }
 
+/// Returns the nearest multiple of `factor` that is `>=` than `num`. Here `factor` must
+/// be a power of 2.
+///
+/// Copied from the arrow crate to make arrow optional
+pub fn round_upto_power_of_2(num: usize, factor: usize) -> usize {

Review comment:
       this was copied from `arrow/src/util/bit_util.rs` (which is fine, I am just pointing it out)

##########
File path: parquet/Cargo.toml
##########
@@ -60,6 +60,7 @@ serde_json = { version = "1.0", features = ["preserve_order"] }
 [features]
 default = ["arrow", "snap", "brotli", "flate2", "lz4", "zstd", "base64"]
 cli = ["serde_json", "base64", "clap"]
+test = []

Review comment:
       I think if you call this feature something other than `test` (`test_common` perhaps) and add it to the `default` list the CI will work
   
   ```diff
   diff --git a/parquet/Cargo.toml b/parquet/Cargo.toml
   index 830e33505e..7ad02298b7 100644
   --- a/parquet/Cargo.toml
   +++ b/parquet/Cargo.toml
   @@ -58,9 +58,9 @@ arrow = { path = "../arrow", version = "6.0.0-SNAPSHOT" }
    serde_json = { version = "1.0", features = ["preserve_order"] }
    
    [features]
   -default = ["arrow", "snap", "brotli", "flate2", "lz4", "zstd", "base64"]
   +default = ["arrow", "snap", "brotli", "flate2", "lz4", "zstd", "base64", "test_common"]
    cli = ["serde_json", "base64", "clap"]
   -test = []
   +test_common = []
    
    [[ bin ]]
    name = "parquet-read"
   diff --git a/parquet/src/util/mod.rs b/parquet/src/util/mod.rs
   index 2c653ceef9..94787a5b85 100644
   --- a/parquet/src/util/mod.rs
   +++ b/parquet/src/util/mod.rs
   @@ -22,9 +22,9 @@ pub mod bit_util;
    mod bit_packing;
    pub mod cursor;
    pub mod hash_util;
   -#[cfg(feature = "test")]
   +#[cfg(feature = "test_common")]
    pub(crate) mod test_common;
   -#[cfg(feature = "test")]
   +#[cfg(feature = "test_common")]
    pub use self::test_common::page_util::{
        DataPageBuilder, DataPageBuilderImpl, InMemoryPageIterator,
    };
   ```
   
   
   

##########
File path: .github/workflows/rust.yml
##########
@@ -118,6 +118,9 @@ jobs:
           cargo run --example dynamic_types
           cargo run --example read_csv
           cargo run --example read_csv_infer_schema
+          (cd parquet && cargo check --no-default-features)

Review comment:
       💯 Thank you for adding the tests




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