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

Allow the creation of String arrays from an interator of &Option<&str> #598

Closed
alamb opened this issue Jul 22, 2021 · 3 comments · Fixed by #680
Closed

Allow the creation of String arrays from an interator of &Option<&str> #598

alamb opened this issue Jul 22, 2021 · 3 comments · Fixed by #680
Labels
enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Jul 22, 2021

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
I am making constant arrays for tests from constant slices

With 1.53 Rust introduces the ability to turn constant slices into iterators. For example. However, the collect for

This does not work:

        let string_array: ArrayRef = Arc::new([Some("foo"), None, Some("bar")].into_iter().collect::<StringArray>());

It produces this error:

error[E0277]: a value of type `GenericStringArray<i32>` cannot be built from an iterator over elements of type `&Option<&str>`
  --> arrow_util/src/display.rs:43:92
   |
43 |         let string_array: ArrayRef = Arc::new([Some("foo"), None, Some("bar")].into_iter().collect::<StringArray>());
   |                                                                                            ^^^^^^^ value of type `GenericStringArray<i32>` cannot be built from `std::iter::Iterator<Item=&Option<&str>>`
   |
   = help: the trait `FromIterator<&Option<&str>>` is not implemented for `GenericStringArray<i32>`

But this does (for primitive arrays):

        let int64_array: ArrayRef = Arc::new([Some(-1), None, Some(2)].into_iter().collect::<Int64Array>());

Instead, for strings we have to do

        let string_array: ArrayRef = Arc::new(vec![Some("foo"), None, Some("bar")].into_iter().collect::<StringArray>());

Describe the solution you'd like
I would like to be able to do (note the lack of vec!):

        let string_array: ArrayRef = Arc::new([Some("foo"), None, Some("bar")].into_iter().collect::<StringArray>());

Perhaps by making the FromIter implementation fancier (right now it hard codes a Option<&str> but the iterator above tives a &Option<&str>: https://github.com/apache/arrow-rs/blob/master/arrow/src/array/array_string.rs#L255-L262

Additional context

Note -- the same sort if issue exists for DictionaryArray as well

Does not work:

let dict_array = [Some("a"), None, Some("b")]
                .into_iter().collect::<DictionaryArray::<Int32Type>>();

Does work

let dict_array = vec![Some("a"), None, Some("b")]
                .into_iter().collect::<DictionaryArray::<Int32Type>>();
@alamb alamb added good first issue Good for newcomers enhancement Any new improvement worthy of a entry in the changelog labels Jul 22, 2021
@jorgecarleitao
Copy link
Member

I am probably missing something. The below does not compile:

let array: StringArray = [Some("foo"), None, Some("bar")]
        .iter()
        .collect();

isn't it enough to use .map(|x| x.as_ref()):

let array: StringArray = [Some("foo"), None, Some("bar")]
        .iter()
        .map(|x| x.as_ref())
        .collect();

it is 20 more characters, but it does not seem that bad?

@alamb
Copy link
Contributor Author

alamb commented Aug 11, 2021

it is 20 more characters, but it does not seem that bad?

I agree there is a straightforward workaround and if you are familiar with the arrow crate this isn't a big deal.

However, I think the fact something that looks like it should work does not increases the barrier to entry for anyone who is first using thearrow crate or those who are not Rust experts yet

@jorgecarleitao
Copy link
Member

that is a great argument 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants