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

Remove DictionaryArray::keys_array method #419

Merged
Merged
Show file tree
Hide file tree
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
21 changes: 3 additions & 18 deletions arrow/src/array/array_dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,26 +70,11 @@ pub struct DictionaryArray<K: ArrowPrimitiveType> {
}

impl<'a, K: ArrowPrimitiveType> DictionaryArray<K> {
/// Return an iterator to the keys of this dictionary.
/// Return an array view of the keys of this dictionary as a PrimitiveArray.
pub fn keys(&self) -> &PrimitiveArray<K> {
&self.keys
}

/// Returns an array view of the keys of this dictionary
pub fn keys_array(&self) -> PrimitiveArray<K> {
let data = self.data_ref();
let keys_data = ArrayData::new(
K::DATA_TYPE,
data.len(),
Some(data.null_count()),
data.null_buffer().cloned(),
data.offset(),
data.buffers().to_vec(),
vec![],
);
PrimitiveArray::<K>::from(keys_data)
}

/// Returns the lookup key by doing reverse dictionary lookup
pub fn lookup_key(&self, value: &str) -> Option<K::Native> {
let rd_buf: &StringArray =
Expand Down Expand Up @@ -379,7 +364,7 @@ mod tests {
let test = vec!["a", "b", "c", "a"];
let array: DictionaryArray<Int8Type> = test.into_iter().collect();

let keys = array.keys_array();
let keys = array.keys();
assert_eq!(&DataType::Int8, keys.data_type());
assert_eq!(0, keys.null_count());
assert_eq!(&[0, 1, 2, 0], keys.values());
Expand All @@ -390,7 +375,7 @@ mod tests {
let test = vec![Some("a"), None, Some("b"), None, None, Some("a")];
let array: DictionaryArray<Int32Type> = test.into_iter().collect();

let keys = array.keys_array();
let keys = array.keys();
assert_eq!(&DataType::Int32, keys.data_type());
assert_eq!(3, keys.null_count());

Expand Down
2 changes: 1 addition & 1 deletion arrow/src/array/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3196,7 +3196,7 @@ mod tests {
assert_eq!(array.is_null(1), true);
assert_eq!(array.is_valid(1), false);

let keys = array.keys_array();
let keys = array.keys();

assert_eq!(keys.value(0), 1);
assert_eq!(keys.is_null(1), true);
Expand Down
4 changes: 2 additions & 2 deletions arrow/src/array/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ where
{
let left = left.as_any().downcast_ref::<DictionaryArray<T>>().unwrap();
let right = right.as_any().downcast_ref::<DictionaryArray<T>>().unwrap();
let left_keys = left.keys_array();
let right_keys = right.keys_array();
let left_keys = left.keys();
let right_keys = right.keys();

let left_values = StringArray::from(left.values().data().clone());
let right_values = StringArray::from(right.values().data().clone());
Expand Down
6 changes: 4 additions & 2 deletions arrow/src/compute/kernels/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,8 @@ fn dictionary_cast<K: ArrowDictionaryKeyType>(
)
})?;

let keys_array: ArrayRef = Arc::new(dict_array.keys_array());
let keys_array: ArrayRef =
Arc::new(PrimitiveArray::<K>::from(dict_array.keys().data().clone()));
let values_array = dict_array.values();
let cast_keys = cast_with_options(&keys_array, to_index_type, &cast_options)?;
let cast_values =
Expand Down Expand Up @@ -1450,7 +1451,8 @@ where
cast_with_options(&dict_array.values(), to_type, cast_options)?;

// Note take requires first casting the indices to u32
let keys_array: ArrayRef = Arc::new(dict_array.keys_array());
let keys_array: ArrayRef =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If primitive arrays were cloneable this would just be Arc::new(dict_array.keys().clone()).

Arc::new(PrimitiveArray::<K>::from(dict_array.keys().data().clone()));
let indicies = cast_with_options(&keys_array, &DataType::UInt32, cast_options)?;
let u32_indicies =
indicies
Expand Down
4 changes: 2 additions & 2 deletions arrow/src/compute/kernels/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ fn sort_string_dictionary<T: ArrowDictionaryKeyType>(
) -> Result<UInt32Array> {
let values: &DictionaryArray<T> = as_dictionary_array::<T>(values);

let keys: &PrimitiveArray<T> = &values.keys_array();
let keys: &PrimitiveArray<T> = values.keys();

let dict = values.values();
let dict: &StringArray = as_string_array(&dict);
Expand Down Expand Up @@ -1028,7 +1028,7 @@ mod tests {
.as_any()
.downcast_ref::<StringArray>()
.expect("Unable to get dictionary values");
let sorted_keys = sorted.keys_array();
let sorted_keys = sorted.keys();

assert_eq!(sorted_dict, dict);

Expand Down
2 changes: 1 addition & 1 deletion arrow/src/compute/kernels/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ where
I: ArrowNumericType,
I::Native: ToPrimitive,
{
let new_keys = take_primitive::<T, I>(&values.keys_array(), indices)?;
let new_keys = take_primitive::<T, I>(values.keys(), indices)?;
let new_keys_data = new_keys.data_ref();

let data = ArrayData::new(
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/util/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ fn dict_array_value_to_string<K: ArrowPrimitiveType>(
) -> Result<String> {
let dict_array = colum.as_any().downcast_ref::<DictionaryArray<K>>().unwrap();

let keys_array = dict_array.keys_array();
let keys_array = dict_array.keys();

if keys_array.is_null(row) {
return Ok(String::from(""));
Expand Down