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

[C++] Add operator[] to some concrete Array implementations? #22660

Closed
asfimport opened this issue Aug 16, 2019 · 7 comments
Closed

[C++] Add operator[] to some concrete Array implementations? #22660

asfimport opened this issue Aug 16, 2019 · 7 comments

Comments

@asfimport
Copy link
Collaborator

asfimport commented Aug 16, 2019

I have occasionally been in situations where I want to pass in e.g. Int32Array or BooleanArray into a template that also can accept STL sequence types like std::vector<bool> – in such cases the main API of interest is []. Would there be interest in aliasing operator[] to return the result of GetView (or to deprecate GetView in favor of operator[])?

Reporter: Wes McKinney / @wesm
Assignee: Pradeep Garigipati / @9prady9

Related issues:

PRs and other links:

Note: This issue was originally created as ARROW-6276. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Francois Saint-Jacques / @fsaintjacques:
I wanted such interface. I think we have to come up with a way to return option<T> and T depending on the null count, we could achieve this with a shim Array wrapper. I'd also like if we could have c++ iterators.

@asfimport
Copy link
Collaborator Author

Antoine Pitrou / @pitrou:
We now have STL iterators in arrow/stl_iterator.h for a couple datatypes (primitive, binary).
We could add syntactic sugar for indexed access, e.g.:

typename IteratorType::ValueType operator[](int64_t i) {
  return *IteratorType(*this, i);
}

@asfimport
Copy link
Collaborator Author

Pradeep Garigipati / @9prady9:
@wesm @pitrou Is the following kind of change that is intended ? or the operator overload should take a scalar of NumericType ?

bool BooleanArray::operator[](int64_t i) const { return GetView(i); }
value_type NumericArray::operator[](int64_t i) const { return GetView(i); }
TypeClass::DayMilliseconds DayTimeIntervalArray::operator[](int64_t i) const { return GetView(i); }
TypeClass::MonthDayNanos MonthDayNanoIntervalArray::operator[](int64_t i) const { return GetView(i); }

@asfimport
Copy link
Collaborator Author

Antoine Pitrou / @pitrou:
See my comment above. So, yes, it's this kind of change.

@asfimport
Copy link
Collaborator Author

Pradeep Garigipati / @9prady9:
The first two sentences of the original description kind of confused me (as in the operator[] indexes one Array using another Int32Array or something like that) even though the last sentence does say the new operator overload should return GetView's result.

Thank you for the clarification. I have raised the PR, please have a look.

@asfimport
Copy link
Collaborator Author

Eduardo Ponce / @edponce:
Now that Array types support {}operator[]{}, should it be used instead of GetView()? If so, then should we substitute all invocations to GetView() and make GetView() a private method which serves only as the implementation of operator[]?

@asfimport
Copy link
Collaborator Author

Antoine Pitrou / @pitrou:
Issue resolved by pull request 11694
#11694

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant