-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Minor: update documentation for PrunableStatistics #16213
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
Conversation
/// vectors represent the containers. | ||
/// The order must match the order of the partition columns in | ||
/// [`PartitionPruningStatistics::partition_schema`]. | ||
/// |
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 was a driveby cleanup -- to add some whitespace for my whitespace OCD
|
||
/// Prune a set of containers represented by their statistics. | ||
/// Each [`Statistics`] represents a container (e.g. a file or a partition of files). | ||
/// |
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 is the real change of the PR -- to reword this description to try and make it easier to understand
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.
lgtm thanks @alamb
/// It is up to the caller to decide what each container represents. For | ||
/// example, they can come from a file (e.g. [`PartitionedFile`]) or a set of of | ||
/// files (e.g. [`FileGroup`]) | ||
/// | ||
/// [`PartitionedFile`]: https://docs.rs/datafusion/latest/datafusion/datasource/listing/struct.PartitionedFile.html | ||
/// [`FileGroup`]: https://docs.rs/datafusion/latest/datafusion/datasource/physical_plan/struct.FileGroup.html |
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!
Which issue does this PR close?
Rationale for this change
Implement suggestions from @xudong963: #16139 (comment)
What changes are included in this PR?
Tweak the docs a bit
Are these changes tested?
Are there any user-facing changes?