ENH: geography constructor from geoarrow #49
+331
−3
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
This reverts commit a01dc4b.
jorisvandenbossche
force-pushed
the
geoarrow-input
branch
from
030fb60
to
b2ff23d
Compare
benbovy
approved these changes
Nov 5, 2024
| The maximum distance in meters that a point must be moved to | ||
| satisfy the planar edge constraint. This is only used if `planar` | ||
| is set to True. | ||
| geometry_encoding : str, default None |
There was a problem hiding this comment.
Suggested change
| geometry_encoding : str, default None | |
| geography_encoding : str, default None |
Not sure which is best, though.
There was a problem hiding this comment.
Yeah, given this is for geoarrow IO and in geoarrow we generally use the "geometry" term, I thought to keep that as well for the keyword ... But either way it is not ideal.
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 depends on paleolimbot/s2geography#23This 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:
from_geoarrowthat 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.__arrow_c_array__, but probably should also support__arrow_c_stream__? (given that a lot of other libraries will often only implement that)s2geography::geoarrow::Readeralready 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)
from_wktthat is a normal vectorized function, but the problem is that pybind11'svectorizedoesn't seem to likestd::stringargument input for a vectorized arg. -> this is done in ENH: add from_wkt/to_wkt functions #50