Skip to content

ARROW-462: [C++] Implement in-memory conversions between non-nested primitive types and DictionaryArray equivalent#812

Closed
xhochy wants to merge 2 commits intoapache:masterfrom
xhochy:ARROW-462
Closed

ARROW-462: [C++] Implement in-memory conversions between non-nested primitive types and DictionaryArray equivalent#812
xhochy wants to merge 2 commits intoapache:masterfrom
xhochy:ARROW-462

Conversation

@xhochy
Copy link
Member

@xhochy xhochy commented Jul 4, 2017

Simple usage:

Array primitive_array;
DictionaryBuilder<Type> builder;
RETURN_NOT_OK(builder.AppendArray(primitive_array));
std::shared_ptr<Array> dict_encoded_array;
RETURN_NOT_OK(builder.Finish(&dict_encoded_array));

…rimitive types and DictionaryArray equivalent

Change-Id: I624667e3ca89a0c0f19b33d3efe5e39146941b61
Status DictionaryBuilder<T, Scalar>::DoubleTableSize() {
template <typename T>
Status DictionaryBuilder<T>::AppendArray(const Array& array) {
const NumericArray<T>& numeric_array = static_cast<const NumericArray<T>&>(array);
Copy link
Member

Choose a reason for hiding this comment

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

You could use PrimitiveArray here and cast the raw_data() to const Scalar*

: DictionaryBuilder<T1>(pool, TypeTraits<T1>::type_singleton()) {}

Status Append(const Scalar& value);
Status AppendArray(const Array& array);
Copy link
Member

Choose a reason for hiding this comment

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

I don't know whether overloaded methods or differently-named methods is better.

As an aside, should start writing more Doxygen docstrings for new APIs (and making a pass over our existing APIs). Please remind me to write more documentation in my PRs, too =)

Copy link
Member Author

Choose a reason for hiding this comment

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

Overloaded methods did sadly not work in combination with the templates. Thus I renamed them to avoid the compiler errors.

@wesm
Copy link
Member

wesm commented Jul 4, 2017

Looks like the tests only failed due to anaconda.org problems

…n internal namespace. Add BinaryDictionaryBuilder convenience

Change-Id: Icb130ec40b6bf43d026749aa142359efea04aca9
@wesm
Copy link
Member

wesm commented Jul 6, 2017

+1, will await build. I made a couple small tweaks

@asfgit asfgit closed this in c398fda Jul 7, 2017
@wesm wesm deleted the ARROW-462 branch July 7, 2017 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants