-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve datastore interface #211
Merged
Merged
Conversation
This file contains 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
Made draft until its made non-breaking |
abeforgit
force-pushed
the
feature/better-datastore
branch
from
February 16, 2022 13:25
4cfbccf
to
e1a7e54
Compare
lagartoverde
approved these changes
Feb 24, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I think maybe we will have to change some of the range strategies in the future but we will see that when using them what makes sense now it may not make sense in a few months. Good job :)
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.
Adds new node query methods which return a more workable response
Changes:
Both of these remain iterable (with for...of), but both offer some convenienve methods over normal sets.
single()
: takes the only element and returns it. Errors if there are more elements in the iterator. Useful in situations where you know there can only be one result.map(mappingFunc)
: normal "apply this func to each item in the iterator" stuffTermMapping also has a
get(termSpec)
method, for when you've matched multiple terms, but you need the nodes for a specific term. Takes the same kind of arguments asdatastore.match
(so concise term syntax, or RDFjs term objects)Another improvement is that the nodes per term are now an array instead of a set, and they are guaranteed to be in document order, and contain no duplicates. The ordering was a piece of information we already had, but was destroyed by using sets to ensure no duplicates.
Also adds a minor improvement to the RangeFactory, setting default offset values for the
fromInNode
method.Lastly, splits up the datastore file into several files, it was starting to become a little too long since it contained 4 class definitions and a bunch of types.