Skip to content
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

warning: methods as_any and next_batch are never used in parquet crate #6143

Closed
alamb opened this issue Jul 28, 2024 · 3 comments · Fixed by #6432
Closed

warning: methods as_any and next_batch are never used in parquet crate #6143

alamb opened this issue Jul 28, 2024 · 3 comments · Fixed by #6432
Labels
bug parquet Changes to the parquet crate

Comments

@alamb
Copy link
Contributor

alamb commented Jul 28, 2024

Describe the bug
While publishing 52.2.0 I noticed this warning: #5998

I have also seen it locally

warning: methods `as_any` and `next_batch` are never used
  --> src/arrow/array_reader/mod.rs:64:8
   |
63 | pub trait ArrayReader: Send {
   |           ----------- methods in this trait
64 |     fn as_any(&self) -> &dyn Any;
   |        ^^^^^^
...
70 |     fn next_batch(&mut self, batch_size: usize) -> Result<ArrayRef> {
   |        ^^^^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: `parquet` (lib) generated 1 warning

To Reproduce

cargo test --test arrow_reader

Expected behavior
No warnings

Additional context

@alamb alamb added bug parquet Changes to the parquet crate labels Jul 28, 2024
@etseidl
Copy link
Contributor

etseidl commented Sep 5, 2024

I've been playing around with this, and found that next_batch is only used in benchmarks and tests, while as_any truly does seem to be completely unused. For next_batch we can add #[cfg(any(feature = "experimental", test))] to silence the warning. For as_any, we could either remove it (it appears unused, and array_reader does not appear to be public), or simply allow dead code if we think as_any might be of use down the road. We could also just throw #[allow(dead_code)] on both. @alamb any preferences here?

@alamb
Copy link
Contributor Author

alamb commented Sep 6, 2024

Thanks @etseidl

I wonder if the issue is that the ArrayReader trait itself is not pub by default (maybe it needs an experimental flag?) - if it was a public trait then the error shouldn't happen

Maybe we can add a

#[cfg(any(feature = "experimental", test))]

To the overall trait?

@etseidl
Copy link
Contributor

etseidl commented Sep 6, 2024

Maybe we can add a

#[cfg(any(feature = "experimental", test))]

To the overall trait?

Tried it and that caused a huge ripple (which I find odd since the whole module is marked experimental, but I think I still don't fully understand the mechanics there 🤷). I'm also puzzled over why the warnings starting appearing in the first place. Was it a code change or just improvements in unused code detection? I haven't wanted to start bisecting to run that thought to ground 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants