-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Minor: improve ParquetOpener docs #12456
Conversation
pub partition_index: usize, | ||
/// Column indexes in `table_schema` needed by the query |
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.
This in particular was unclear when @itsjunetime and I read the code (were the indexes in terms of file_schema or table_schema?) and was the impetus for this PR
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.
Thank you @alamb! The descriptions are helpful, especially those for bool values!
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.
Nice, WDYT about enabling the missing_docs
lint so every pub will force forced to be documented ?
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e486601506c2493123d2b4d9cca24727 |
I personally think it is a good idea, though I predict that if we do that we'll still end up with functions that are like "ParquetOpener: opens parquet" any other similar documentation that isn't particularly helpful. But it might encourage enough that is better than no documentation 👍 |
Thanks for the review @appletreeisyellow and @comphead |
I'll try to play on some small crate first, enabling the documentation and putting |
Which issue does this PR close?
Part of #4028
Rationale for this change
While working with @itsjunetime on #12135 we spent a while reviewing the code in
ParquetOpener
and I realized that the documentation could be improvedWhat changes are included in this PR?
Improve the documentation
Are these changes tested?
By CI
Are there any user-facing changes?
No functional changes, just docs