Skip to content

Conversation

@TheNeuralBit
Copy link
Member

Added an optional batch to Vector.get, as well as a *batches iterator. The most significant changes are in ListVector and IndexVector, because I removed the returnWithBatchIndex argument and made some tweaks to the way offset vectors are handled (Rather than subtracting batch when accessing data, use length - 1 to reduce `index while iterating).

@wesm
Copy link
Member

wesm commented Oct 14, 2017

Can you add some tests? Are there coverage tools available for Typescript?

@trxcllnt
Copy link
Contributor

trxcllnt commented Oct 15, 2017

@wesm yep we have coverage ready to go, but commented out in the CI script until we set up something like coveralls.
@TheNeuralBit sorry I haven't responded to your comments in ARROW-1652 yet, I'll do that right now.

@wesm
Copy link
Member

wesm commented Oct 19, 2017

@TheNeuralBit could you add some unit tests and update the PR title with the JIRA number?

@TheNeuralBit TheNeuralBit changed the title WIP: [JS] Add optional batch hint to Vector.get ARROW-1652 [JS - WIP] Add optional batch hint to Vector.get Oct 19, 2017
@TheNeuralBit
Copy link
Member Author

@wesm I think this may be irrelevant now based on our new plans for the Vector class
@trxcllnt what do you think? Maybe ARROW-1652 should be changed to "add batch vector accessor" and include your planned changes

@trxcllnt
Copy link
Contributor

@TheNeuralBit yeah that works for me, that way it's still tracked

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