Skip to content
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

ENH: geography constructor from geoarrow #49

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

jorisvandenbossche
Copy link
Collaborator

@jorisvandenbossche jorisvandenbossche commented Oct 3, 2024

This depends on paleolimbot/s2geography#23

This also introduces the question: how to we deal with functions that depend on a specific version of s2geography? Short term we can just ensure we do a upstream release and require the latest version, but longer term we might need to have some mechanism for this? -> added some version definitions that can be used in C++

Some other notes:

  • This PR adds a from_geoarrow that accepts any Arrow-compatible array object (through the Arrow PyCapsule interface). As a result, this is not a numpy vectorized function that will take any dimension of input, it's purely a 1D -> 1D function.
    • At the moment this PR also only adds support for __arrow_c_array__, but probably should also support __arrow_c_stream__? (given that a lot of other libraries will often only implement that)
  • The current upstream implementation of the s2geography::geoarrow::Reader already supports WKT, WKB and native (nested coordinates) encoding, so those work out of the box here as well.
    Right now I only accept Arrow input with an extension type and let the Reader figure out the encoding, but we could also add a argument to let the user specify "WKT"/"WKB" and then accept a plain Arrow object (without geoarrow extension type)
  • I did try to add a specific from_wkt that is a normal vectorized function, but the problem is that pybind11's vectorize doesn't seem to like std::string argument input for a vectorized arg. -> this is done in ENH: add from_wkt/to_wkt functions #50

@jorisvandenbossche jorisvandenbossche marked this pull request as ready for review October 25, 2024 18:35
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.

1 participant