-
Notifications
You must be signed in to change notification settings - Fork 8
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: add projection support in geoarrow output #69
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#include "projections.hpp" | ||
|
||
#include <s2/s2projections.h> | ||
#include <s2geography.h> | ||
|
||
#include "geography.hpp" | ||
#include "pybind11.hpp" | ||
|
||
namespace py = pybind11; | ||
namespace s2geog = s2geography; | ||
using namespace spherely; | ||
|
||
void init_projections(py::module& m) { | ||
py::class_<Projection>(m, "Projection") | ||
.def("lnglat", &Projection::lnglat) | ||
.def("pseudo_mercator", &Projection::pseudo_mercator) | ||
.def("orthographic", &Projection::orthographic); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
#ifndef SPHERELY_PROJECTIONS_H_ | ||
#define SPHERELY_PROJECTIONS_H_ | ||
|
||
#include <s2/s2latlng.h> | ||
#include <s2/s2projections.h> | ||
#include <s2geography.h> | ||
|
||
#include "pybind11.hpp" | ||
|
||
namespace py = pybind11; | ||
namespace s2geog = s2geography; | ||
using namespace spherely; | ||
|
||
class Projection { | ||
public: | ||
Projection(std::shared_ptr<S2::Projection> projection) | ||
: m_s2_projection(std::move(projection)) {} | ||
|
||
std::shared_ptr<S2::Projection> s2_projection() { | ||
return m_s2_projection; | ||
} | ||
|
||
static Projection lnglat() { | ||
return Projection(std::move(s2geog::lnglat())); | ||
} | ||
static Projection pseudo_mercator() { | ||
return Projection(std::move(s2geog::pseudo_mercator())); | ||
} | ||
static Projection orthographic(double longitude, double latitude) { | ||
return Projection( | ||
std::move(s2geog::orthographic(S2LatLng::FromDegrees(latitude, longitude)))); | ||
} | ||
|
||
private: | ||
std::shared_ptr<S2::Projection> m_s2_projection; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 ? |
||
}; | ||
|
||
#endif // SPHERELY_PROJECTIONS_H_ |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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