alamb commented on code in PR #10104:
URL: https://github.com/apache/arrow-rs/pull/10104#discussion_r3714644835
##########
parquet/src/basic.rs:
##########
@@ -1054,11 +1054,30 @@ impl ColumnOrder {
///
/// `is_type_defined` indicates whether the column order for this type is
/// [`ColumnOrder::TYPE_DEFINED_ORDER`].
+ ///
+ /// It is now prefered to obtain this via [`Self::sort_order`].
+ #[deprecated(since = "60.0.0", note = "use `ColumnOrder::sort_order`
instead")]
Review Comment:
Since most of this PR changes `sort_order_for_type` to
`get_sort_order_for_type` I at first thought this message should say
```rust
#[deprecated(since = "60.0.0", note = "use
`ColumnOrder::get_sort_order_for_type` instead")]
```
But now I see we seem to be consolidating into just using `ColumnOrder`
which is good 👍
##########
parquet/src/basic.rs:
##########
@@ -1054,11 +1054,30 @@ impl ColumnOrder {
///
/// `is_type_defined` indicates whether the column order for this type is
/// [`ColumnOrder::TYPE_DEFINED_ORDER`].
+ ///
+ /// It is now prefered to obtain this via [`Self::sort_order`].
Review Comment:
```suggestion
/// It is now preferred to obtain this via [`Self::sort_order`].
```
##########
parquet/src/basic.rs:
##########
@@ -1054,11 +1054,30 @@ impl ColumnOrder {
///
/// `is_type_defined` indicates whether the column order for this type is
/// [`ColumnOrder::TYPE_DEFINED_ORDER`].
+ ///
+ /// It is now prefered to obtain this via [`Self::sort_order`].
+ #[deprecated(since = "60.0.0", note = "use `ColumnOrder::sort_order`
instead")]
Review Comment:
I wonder if we should document how all the orders relate to each other a bit
more
Like it seems there is a way to get a ColumnOrder for a physical type 🤔
```rust
/// Returns the `ColumnOrder` for a physical/logical type.
pub fn column_order_for_type(
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
