ENH: add projection support in geoarrow output #69
Conversation
jorisvandenbossche
force-pushed
the
projections
branch
from
ee0178b
to
edb3b29
Compare
jorisvandenbossche
force-pushed
the
projections
branch
from
edb3b29
to
ae00163
Compare
|
OK, rebased this on |
| } | ||
|
|
||
| private: | ||
| std::shared_ptr<S2::Projection> m_s2_projection; |
There was a problem hiding this comment.
Not sure why a shared pointer is needed (apart from that this is what is used in s2geography), but that's not a big deal anyway. Feel free to ignore this comment.
There was a problem hiding this comment.
We indeed need a shared pointer to pass to s2geography (and it is also what is returned by the s2geography constructors), so then I though that might be easier to just store the shared_ptr on the object, instead of unpacking it / packing it again in a shared_ptr ?
| @@ -338,6 +340,10 @@ void init_geoarrow(py::module& m) { | |||
| can be a ``pyarrow.DataType`` or ``pyarrow.Field``, and you can | |||
| use the ``geoarrow.pyarrow`` package to construct such geoarrow | |||
| extension types. | |||
| projection : spherely.Projection, default Projection.lnglat() | |||
There was a problem hiding this comment.
| projection : spherely.Projection, default Projection.lnglat() | |
| projection : :py:class:`Projection`, default :py:meth:`Projection.lnglat` |
Although maybe both are rendered as hyperlinks by sphinx? Either way it needs Projection be added to the API reference doc.
There was a problem hiding this comment.
So I added it to the API docs to see if the linking would go automatically, which I suppose will be the case>? (at least for Geography it works)
But our docs get build with the released s2geography, of course, so this is not yet included
Follow-up on #53