Conversation
stavrospapadopoulos
approved these changes
Jul 25, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a new Query class in R, wrapping the corresponding library element. Adding it allows us to rewrite a lot of the examples to make them simpler and "R idiomatic".
This set of change is mostly complete but still needs treatment for variable length (i.e. char / string) column types which also needs a helper function to go back and forth between normal vectors of strings and our "package" char vector and offsets vector. I may leave that for a later PR.
Code from the examples should also be carried over to the unit tests. I should get to that tomorrow. Labeling this as a 'draft' for now.This is now done.Some of the work for this was done a good week ago and has been rebased twice with other changes to the main branch.
The red crosses below are "simply" due to not skipping a few tests for TileDB < 2.0.0. I.e. those runs each passed on the four other instances tested.