Skip to content
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

Doctests for DecimalArray. #414

Merged
merged 3 commits into from
Jun 14, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions arrow/src/array/array_binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,30 @@ impl Array for FixedSizeBinaryArray {
}

/// A type of `DecimalArray` whose elements are binaries.
///
/// # Examples
///
/// ```
/// use arrow::array::{Array, DecimalArray, DecimalBuilder};
/// use arrow::datatypes::DataType;
/// let mut builder = DecimalBuilder::new(30, 23, 6);
///
/// builder.append_value(8_887_000_000).unwrap();
/// builder.append_null().unwrap();
/// builder.append_value(-8_887_000_000).unwrap();
/// let decimal_array: DecimalArray = builder.finish();
///
/// assert_eq!(&DataType::Decimal(23, 6), decimal_array.data_type());
/// assert_eq!(8_887_000_000, decimal_array.value(0));
/// assert_eq!("8887.000000", decimal_array.value_as_string(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

/// assert_eq!(3, decimal_array.len());
/// assert_eq!(1, decimal_array.null_count());
/// assert_eq!(32, decimal_array.value_offset(2));
/// assert_eq!(16, decimal_array.value_length());
/// assert_eq!(23, decimal_array.precision());
/// assert_eq!(6, decimal_array.scale());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alippai has an improvement for string formatting decimal values in #406

If he adds the method to DecimalArray in #406 (comment) it would be neat to also add it to the doc test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alamb I have followed your suggestion to replace the example with one that uses DecimalBuilder instead. If I am following correctly, I won't be able to really use any of #406 there will I?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll be able to add assert_eq!("8887.000000", decimal_array.value_as_string(0)); when I finish the PR. Also I'd keep the assert_eq!(8_887_000_000, decimal_array.value(0)); example (to show the values are i128 values)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alippai -- I will hold off updating this PR until #406 is in // @alamb

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@novemberkilo now it's merged, thanks for your patience

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alippai - I have added those doctests per your suggestion. // @alamb

/// ```
///
pub struct DecimalArray {
data: ArrayData,
value_data: RawPtrBox<u8>,
Expand Down