Skip to content

Conversation

@bkietz
Copy link
Member

@bkietz bkietz commented Sep 5, 2019

With #5077 (or possibly #4949) behavior with dictionary arrays changed, leaving the explicit call to DictionaryEncode() redundant @wesm

@wesm
Copy link
Member

wesm commented Sep 5, 2019

Possible to have a unit test for this?

@wesm
Copy link
Member

wesm commented Sep 6, 2019

This is also coming up in ARROW-6435. I'll add a unit test there.

@kszucs kszucs changed the title ARROW-6434: [C++] Don't try to dictionary encode dictionary arrays ARROW-6475: [C++] Don't try to dictionary encode dictionary arrays Sep 6, 2019
@kszucs kszucs closed this in b8ebc9d Sep 6, 2019
@wesm
Copy link
Member

wesm commented Sep 6, 2019

@kszucs why did you merge this?

@wesm
Copy link
Member

wesm commented Sep 6, 2019

This patch has no unit tests and I addressed the fix and the unit test in #5302

@kszucs
Copy link
Member

kszucs commented Sep 6, 2019

Your builds were passing including the test. Sorry, I was hesitating whether I should close it, now I know the answer.

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.

3 participants