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 2022/02/01 14:09:14 UTC
[GitHub] [arrow-rs] alamb opened a new issue #1254: Fix all clippy lints in parquet crate
alamb opened a new issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254
**Describe the bug**
Due to "historical reasons" there are several clippy lints that are disabled in the parquet crate
https://github.com/apache/arrow-rs/blob/master/parquet/src/lib.rs#L18-L36
```rust
#![allow(incomplete_features)]
#![allow(dead_code)]
#![allow(non_camel_case_types)]
#![allow(
clippy::approx_constant,
clippy::cast_ptr_alignment,
clippy::float_cmp,
clippy::float_equality_without_abs,
clippy::from_over_into,
clippy::many_single_char_names,
clippy::needless_range_loop,
clippy::new_without_default,
clippy::or_fun_call,
clippy::same_item_push,
clippy::too_many_arguments,
clippy::transmute_ptr_to_ptr,
clippy::upper_case_acronyms,
clippy::vec_init_then_push
)]
```
It would be great to clean up the code to pass these lints for tidiness
**To Reproduce**
Remove one of the `#[allow]` lines above, run `clippy`
**Expected behavior**
Clippy runs cleanly without blank `#allow` across the whole crate
**Additional context**
Add any other context about the problem here.
--
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
[GitHub] [arrow-rs] soham4abc commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
soham4abc commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1026896987
I Would like to work on the issue... If possible please assign
--
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
[GitHub] [arrow-rs] alamb commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
alamb commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1030049291
> There are many dead code and unused public functions. I am not sure whether to clean them up.
I recommend doing this issue in a few PRs -- in the first take care of the easier lints, and then in follow ons we can sort out dead code / public function nonsense together
--
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
[GitHub] [arrow-rs] alamb commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
alamb commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1054584026
Sorry for the late reply @HaoYang670
> Lots of pub members are detected as dead code. I am not sure whether to clean them up. For example:
If the code is dead I think we should remove it. One way for `pub` functions to be dead is if the module they are defined in is not `pub`.
I am not sure how you are running clippy, but it may also be that some of these structs are only used when certain features of the `parquet` crate are enabled.
I believe our CI system has good coverage so feel free to try removing them and if the CI passes I think it is a good 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
[GitHub] [arrow-rs] alamb commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
alamb commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1058411284
Clippy is run like this:
https://github.com/apache/arrow-rs/blob/master/.github/workflows/rust.yml#L211-L250
```
cargo clippy --features test_common --features prettyprint --features=async --all-targets --workspace -- -D warnings
```
Perhaps we can also add the `experimental` features to this list?
--
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
[GitHub] [arrow-rs] HaoYang670 commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1029598846
There are many dead code and unused public functions. I am not sure whether to clean them up.
--
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
[GitHub] [arrow-rs] HaoYang670 edited a comment on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
HaoYang670 edited a comment on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1057983045
> but it may also be that some of these structs are only used when certain features of the `parquet` crate are enabled.
Thank you @alamb. There are some experimental mods under `parquet`. And I have to run `cargo clippy --features experimental` to make them public. But I am not sure whether our CI adds this flag.
--
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
[GitHub] [arrow-rs] HaoYang670 commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1045563502
Lots of `pub` members are detected as dead code. I am not sure whether to clean them up. For example:
```rust
warning: enum is never used: `LevelDecoder`
--> parquet/src/encodings/levels.rs:151:10
|
151 | pub enum LevelDecoder {
| ^^^^^^^^^^^^
warning: associated function is never used: `v1`
--> parquet/src/encodings/levels.rs:165:12
|
165 | pub fn v1(encoding: Encoding, max_level: i16) -> Self {
| ^^
warning: associated function is never used: `v2`
--> parquet/src/encodings/levels.rs:180:12
|
180 | pub fn v2(max_level: i16) -> Self {
| ^^
warning: associated function is never used: `set_data`
--> parquet/src/encodings/levels.rs:194:12
|
194 | pub fn set_data(&mut self, num_buffered_values: usize, data: ByteBufferPtr) -> usize {
| ^^^^^^^^
warning: associated function is never used: `set_data_range`
--> parquet/src/encodings/levels.rs:221:12
|
221 | pub fn set_data_range(
| ^^^^^^^^^^^^^^
warning: associated function is never used: `is_data_set`
--> parquet/src/encodings/levels.rs:242:12
|
242 | pub fn is_data_set(&self) -> bool {
| ^^^^^^^^^^^
warning: associated function is never used: `get`
--> parquet/src/encodings/levels.rs:254:12
|
254 | pub fn get(&mut self, buffer: &mut [i16]) -> Result<usize> {
| ^^^
warning: associated function is never used: `buffer`
--> parquet/src/encodings/rle.rs:171:12
|
171 | pub fn buffer(&self) -> &[u8] {
| ^^^^^^
warning: associated function is never used: `get`
--> parquet/src/encodings/rle.rs:358:12
|
358 | pub fn get<T: FromBytes>(&mut self) -> Result<Option<T>> {
```
--
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
[GitHub] [arrow-rs] HaoYang670 commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1057983045
> but it may also be that some of these structs are only used when certain features of the `parquet` crate are enabled.
Thank you @alamb. There are some experimental mods under `parquet`. And I must add `--features experimental` to make them public. But I am not sure whether our CI adds this flag.
--
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
[GitHub] [arrow-rs] soham4abc removed a comment on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
soham4abc removed a comment on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1026896987
I Would like to work on the issue... If possible please assign
--
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
[GitHub] [arrow-rs] HaoYang670 commented on issue #1254: Fix all clippy lints in parquet crate
Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on issue #1254:
URL: https://github.com/apache/arrow-rs/issues/1254#issuecomment-1028578608
I'd like to have a try if no one else has been doing it!
Rust Clippy is really interesting!
--
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