[GitHub] [arrow-rs] alamb opened a new pull request, #4615: Minor: improve docs and add example for lexicographical_partition_ranges

["alamb (via GitHub)" <gi...@apache.org>]

alamb opened a new pull request, #4615:
URL: https://github.com/apache/arrow-rs/pull/4615

   # Which issue does this PR close?
   re https://github.com/apache/arrow-rs/pull/4575
   
   # Rationale for this change
    
   While reviewing https://github.com/apache/arrow-rs/pull/4575 I wanted to make sure I really understood what was going on, so I made some pictures
   
   # What changes are included in this PR?
   
   Add docstrings with ASCII art diagram and code example
   
   <!--
   <img width="784" alt="Screenshot 2023-08-01 at 7 51 15 AM" src="https://github.com/apache/arrow-rs/assets/490673/e65ac71a-5a63-4031-ae2d-32423e27bb2c">
   
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   # Are there any user-facing changes?
   docs
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!---
   If there are any breaking changes to public APIs, please add the `breaking change` label.
   -->
   


-- 
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] tustvold commented on a diff in pull request #4615: Minor: improve docs and add example for lexicographical_partition_ranges

["tustvold (via GitHub)" <gi...@apache.org>]

tustvold commented on code in PR #4615:
URL: https://github.com/apache/arrow-rs/pull/4615#discussion_r1281787103


##########
arrow-ord/src/partition.rs:
##########
@@ -22,15 +22,79 @@ use arrow_schema::ArrowError;
 use std::cmp::Ordering;
 use std::ops::Range;
 
-/// Given a list of already sorted columns, find partition ranges that would partition
-/// lexicographically equal values across columns.
+/// Given a list of already sorted columns, returns [`Range`]es that
+/// partition the input such that each partition has equal values
+/// across sort columns.
 ///
-/// Here LexicographicalComparator is used in conjunction with binary
-/// search so the columns *MUST* be pre-sorted already.
+/// Returns an error if no columns are specified or all columns do not
+/// have the same number of rows.
 ///
-/// The returned vec would be of size k where k is cardinality of the sorted values; Consecutive
-/// values will be connected: (a, b) and (b, c), where start = 0 and end = n for the first and last
-/// range.
+/// Returns an iterator with `k` items where `k` is cardinality of the
+/// sort values: Consecutive values will be connected: `(a, b)` and `(b,
+/// c)`, where `start = 0` and `end = n` for the first and last range.

Review Comment:
   ```suggestion
   /// Returns an iterator with `k` items where `k` is cardinality of the
   /// sort values. Consecutive ranges will be contiguous: `(a, b)` and `(b,
   /// c)`, where `start = 0` and `end = n` for the first and last range.
   ```



-- 
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