-
Notifications
You must be signed in to change notification settings - Fork 22
Attempt to expose S2RegionTermIndexer #78
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
base: master
Are you sure you want to change the base?
Attempt to expose S2RegionTermIndexer #78
Conversation
Thanks for the PR! Can you talk a little bit about your use case that you're trying to achieve with this? We might be able to get away with existing primitives in the meantime while we try and find bandwidth on the team to build this out |
Sure! Thanks for the reply. We want to see if we can integrate S2 better with our database indexes and were looking at the The comment on the actual C++ file itself actually describes this pretty well too: https://github.com/google/s2geometry/blob/master/src/s2/s2region_term_indexer.cc We basically just want to expose I'm happy to try to do this work, but there's a bit of a learning curve I need to overcome so any pointers would be great. The current error I'm trying to overcome is:
I thought about just implementing those functions in TypeScript as they aren't that complicated, but I figured it'd be better to use the actual official implementation to avoid bugs. |
That makes sense. I believe the index in question is for in-memory use (rather than in the DB), so you could use it to build a very fast geo service, but it might not be what you're looking for exactly. If you're blocked on this, you can actually make use of the existing The process would be something along the lines of:
you can do something similar with a radius query. you would use the region coverer around the radius for s2LevelMin -> s2LevelMax and query all those tokens. |
We have a need to use S2RegionTermIndexer and this hasn't been exposed in this library.
This is an attempt to bring that in although it's very incomplete and I am currently unable to get past a runtime error caused by the line:
I'm not a C++ dev nor have I used Napi or any other methods of binding C++ binaries to node.
@jkao Any suggestions on why this is happening and how to progress this are welcomed! (Nice work on this btw)