-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add optional left/right parameter to GeoJSON #8978
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
Conversation
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 :) |
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