Skip to content

Conversation

@BryanCutler
Copy link
Owner

Addressed some feedback from the PR. I'll merge these changes unless there are comments, cc @wesm @icexelloss

  • renamed to ArrowConverters
  • encapsulated Arrow usage in ArrowConverters
  • defined ArrowPayload as Iterator of ArrowRecordBatches
  • minor cleanup

@icexelloss
Copy link
Collaborator

LGTM

Copy link

@wesm wesm left a comment

Choose a reason for hiding this comment

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

LGTM also, thank you

@wesm
Copy link

wesm commented Jan 30, 2017

@BryanCutler there are some API changes in pyarrow, I'm writing a patch now to fix

@BryanCutler
Copy link
Owner Author

Ok thanks, I'll make sure to get that updated in the PR also

@wesm
Copy link

wesm commented Jan 30, 2017

Just waiting on Spark to compile to check that things still work =S

BryanCutler added a commit that referenced this pull request Jan 31, 2017
defined ArrowPayload and encapsulated Arrow classes in ArrowConverters

addressed some minor comments in code review

closes #21
BryanCutler added a commit that referenced this pull request Feb 23, 2017
defined ArrowPayload and encapsulated Arrow classes in ArrowConverters

addressed some minor comments in code review

closes #21
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.

4 participants