-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove lint issues in parquet-related code. #9375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2bb71a6 to
efe310e
Compare
* CacheOptions: `use` in arrow/push_decoder/reader_builder/mod.rs was unused. But removing it triggers a doc complaint in array_reader.rs. I fix it by spelling out `CacheOptions` at one usage site. * ExtensionType import was unused in arrow/schema/extension.rs when compiling with default features. Only `use` it inside the feature-guarded code paths. * Prefix unused function arguments with `_`. * Some code in parquet/tests/arrow_reader/io/mod.rs is unused. As I lack knowledge of the context, I just add just add #[allow(dead_code)].
| //! Arrow [`arrow_schema::extension::ExtensionType`]s. | ||
| //! | ||
| //! Extension types are represented using the metadata from Arrow [`Field`]s | ||
| //! with the key "ARROW:extension:name". | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| //! Arrow [`ExtensionType`]s. | |
| //! | |
| //! Extension types are represented using the metadata from Arrow [`Field`]s | |
| //! with the key "ARROW:extension:name". | |
| //! | |
| //! [`ExtensionType`]: arrow_schema::extension::ExtensionType | |
Maybe we can do it like this to keep it clean
| ops: Arc<OperationLog>, | ||
| /// The (pre-parsed) parquet metadata for this file | ||
| // TODO: this is unused; consider removing it. | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe for this dead code we can use it in a test somewhere to assert something useful? 🤔
| match parquet_logical_type { | ||
| #[cfg(feature = "variant_experimental")] | ||
| LogicalType::Variant { .. } => { | ||
| arrow_field.try_with_extension_type(parquet_variant_compute::VariantType)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could rebind in each of these arms like so?
let mut arrow_field = arrow_field;I usually prefer to use expect instead of allow but we'd need to cfg it with three different features here, so maybe its easier to just narrow the mut scope
CacheOptions:
usein arrow/push_decoder/reader_builder/mod.rs wasunused. But removing it triggers a doc complaint in array_reader.rs.
I fix it by spelling out
CacheOptionsat one usage site.ExtensionType import was unused in arrow/schema/extension.rs when
compiling with default features. Only
useit inside thefeature-guarded code paths.
Prefix unused function arguments with
_.Some code in parquet/tests/arrow_reader/io/mod.rs is unused. As I lack
knowledge of the context, I just add just add #[allow(dead_code)].
Rationale for this change
rust-analyzer bugs me about those unused symbols & co.