Support geo label position as runtime field#86154
Merged
craigtaverner merged 12 commits intoelastic:masterfrom May 10, 2022
Merged
Support geo label position as runtime field#86154craigtaverner merged 12 commits intoelastic:masterfrom
craigtaverner merged 12 commits intoelastic:masterfrom
Conversation
Collaborator
|
Hi @craigtaverner, I've created a changelog YAML for you. |
Contributor
Author
|
@elasticmachine run elasticsearch-ci/part-2 |
Collaborator
|
Pinging @elastic/clients-team (Team:Clients) |
iverase
reviewed
May 5, 2022
server/src/main/java/org/elasticsearch/script/field/GeoPointDocValuesField.java
Outdated
Show resolved
Hide resolved
We simplified this to always use the middle of the first tree geometry. The use of the centroid can be done higher up in the stack. For painless, this means in the user written script. For the REST API we will add that logic on the server side.
Now that we no longer use the centroid for polygon labels, but the middle of the first triangle.
This time without rounding error
And more tests, including testing intersects for all label positions.
bfcd82b to
57c977d
Compare
iverase
reviewed
May 6, 2022
Contributor
iverase
left a comment
There was a problem hiding this comment.
Just have a small comment about how we handle an error. Otherwise let's get CI green and it can be approved.
...main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java
Outdated
Show resolved
Hide resolved
4ae19a0 to
ccf30cf
Compare
The centroid is unlikely to be one of the points, so we choose a point closest to the middle of the bounding box.
The changes to expose GeoShapeQueryable.toQuantizeLuceneGeometry to external use also caused exception causes to be one level deeper. This fix just brings the underlying message up one level to get tests to pass. While we could have also changed tests to assert on causes a level deeper, this fix seems more backwards compatible, just in case anyone else is relying on causes.
ccf30cf to
74dc54b
Compare
stu-elastic
approved these changes
May 9, 2022
Contributor
stu-elastic
left a comment
There was a problem hiding this comment.
Scripting looks good.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
There is a need to provide sensibly calculated label positions for polygons and lines in Kibana maps (as requested in #86044). A very convenient way of satisfy this need is through a runtime field that the rest API can make use of when labels are requested. This has the advantage of providing painless access to the label position as well. This PR provides the runtime field.