Rich-T-kid commented on code in PR #10730:
URL: https://github.com/apache/arrow-rs/pull/10730#discussion_r3806118824
##########
arrow-schema/src/ffi.rs:
##########
@@ -133,16 +133,21 @@ unsafe extern "C" fn release_schema(schema: *mut
FFI_ArrowSchema) {
impl FFI_ArrowSchema {
/// create a new [`FFI_ArrowSchema`]. This fails if the fields'
- /// [`DataType`] is not supported.
- ///
- /// # Panics
- ///
- /// Panics if `format` contains an interior nul byte
Review Comment:
nit: I think replacing the # Panics with an error would be neat
##########
arrow-data/src/transform/mod.rs:
##########
@@ -763,17 +772,15 @@ impl<'a> MutableArrayData<'a> {
/// Extends the in progress array with a region of the input arrays.
///
/// # Panics
- /// This function panics if there is an invalid index,
- /// i.e. `index` >= the number of source arrays,
- /// `end` > the length of the `index`th array,
- /// or the offset type overflows (e.g. more than 2 GiB in a `StringArray`).
+ /// This function panics for the same reasons [`Self::try_extend`] returns
an error:
Review Comment:
same thing here, its easier for users to use intellisense to hover over this
function to see the error cases then to jump through file to read
`[`Self::try_extend`] ` doc comment
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -409,28 +409,28 @@ impl VariantArray {
/// Use `try_value` if you need to handle conversion errors gracefully.
///
/// # Panics
- /// * if the index is out of bounds
- /// * if the array value is null
- /// * if `try_value` returns an error.
+ /// Panics if [`VariantArray::try_value`] returns an error, e.g. if the
index is out of bounds.
Review Comment:
```suggestion
/// Panics if the index is out of bounds.
```
##########
arrow-data/src/transform/mod.rs:
##########
@@ -801,15 +817,13 @@ impl<'a> MutableArrayData<'a> {
///
/// # Panics
///
- /// Panics if [`MutableArrayData`] not created with `use_nulls` or
nullable source arrays,
- /// or if the run-end counter overflows for `RunEndEncoded` arrays.
+ /// Panics for the same reasons [`Self::try_extend_nulls`] returns an
error.
Review Comment:
nit: I think its better to write this out, not super important
```suggestion
/// Panics if this [`MutableArrayData`] was not created with `use_nulls`
and none
/// of the source arrays are nullable
```
--
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]
