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
Add optional left/right parameter to GeoJSON #8978
Conversation
@@ -246,7 +252,7 @@ defines the following vertex ordering: | |||
|
|||
For polygons that do not cross the dateline, vertex order will not matter in | |||
Elasticsearch. For polygons that do cross the dateline, Elasticsearch requires | |||
vertex orderinging comply with the OGC specification. Otherwise, an unintended polygon | |||
vertex ordering comply with the OGC specification. Otherwise, an unintended polygon |
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.
vertex ordering TO comply
I can just comment on the intent (LGTM) and the docs (minor comments left) but you'll need to find somebody else to review the code :) |
@@ -265,6 +272,24 @@ OGC standards to eliminate ambiguity resulting in a polygon that crosses the dat | |||
} | |||
-------------------------------------------------- | |||
|
|||
An `orientation` parameter can be defined when setting the geo_shape mapping (see <<geo-shape-mapping-options>>). This will define vertex | |||
order for the index. It can also be overridden on each document. The following is an example for overriding |
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.
What is "index" here? Should this be "field" or "shape" or "coordinates"?
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.
Good catch! Is this any clearer: "This will define vertex order for the coordinate list on the mapped geo_shape field"?
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.
Yes much better.
@nknize I left some comments. |
Good comments Ryan! Really appreciate it. I made some updates based on your feedback. Ready for another review! |
I'm confused why |
Good point Ryan. We can used the java doc for the enum values to describe On Monday, 22 December 2014, Ryan Ernst notifications@github.com wrote:
|
PR #8978 included 4 unnecessary enumeration values ('cw', 'clockwise', 'ccw', 'counterclockwise'). Since the ShapeBuilder.parse method handles these as strings and maps them to LEFT and RIGHT enumerators, respectively, their enumeration counterpart is unnecessary. This minor change adds 4 static convenience variables (COUNTER_CLOCKWISE, CLOCKWISE, CCW, CW) for purposes of the API and removes the unnecessary values from the Orientation Enum. closes #9035
PR #8978 included 4 unnecessary enumeration values ('cw', 'clockwise', 'ccw', 'counterclockwise'). Since the ShapeBuilder.parse method handles these as strings and maps them to LEFT and RIGHT enumerators, respectively, their enumeration counterpart is unnecessary. This minor change adds 4 static convenience variables (COUNTER_CLOCKWISE, CLOCKWISE, CCW, CW) for purposes of the API and removes the unnecessary values from the Orientation Enum. closes #9035
PR #8978 included 4 unnecessary enumeration values ('cw', 'clockwise', 'ccw', 'counterclockwise'). Since the ShapeBuilder.parse method handles these as strings and maps them to LEFT and RIGHT enumerators, respectively, their enumeration counterpart is unnecessary. This minor change adds 4 static convenience variables (COUNTER_CLOCKWISE, CLOCKWISE, CCW, CW) for purposes of the API and removes the unnecessary values from the Orientation Enum. closes #9035
merged from 77a7ef2 |
PR elastic#8978 included 4 unnecessary enumeration values ('cw', 'clockwise', 'ccw', 'counterclockwise'). Since the ShapeBuilder.parse method handles these as strings and maps them to LEFT and RIGHT enumerators, respectively, their enumeration counterpart is unnecessary. This minor change adds 4 static convenience variables (COUNTER_CLOCKWISE, CLOCKWISE, CCW, CW) for purposes of the API and removes the unnecessary values from the Orientation Enum. closes elastic#9035
This feature adds an optional orientation parameter to the GeoJSON document and geo_shape mapping enabling users to explicitly define how they want Elasticsearch to interpret vertex ordering. The default uses the right-hand rule (counterclockwise for outer ring, clockwise for inner ring) complying with OGC Simple Feature Access standards. The parameter can be explicitly specified for an entire index using the geo_shape mapping by adding "orientation":{"left"|"right"|"cw"|"ccw"|"clockwise"|"counterclockwise"} and/or overridden on each insert by adding the same parameter to the GeoJSON document.
closes #8764