Skip to content

[Maps] geo_shape fit to bounds#64303

Merged
nreese merged 5 commits intoelastic:masterfrom
nreese:fit_to_bounds_geo_shape
Apr 27, 2020
Merged

[Maps] geo_shape fit to bounds#64303
nreese merged 5 commits intoelastic:masterfrom
nreese:fit_to_bounds_geo_shape

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented Apr 23, 2020

Closes #33509

Elasticsearch has added geo_bounds aggregation support for geo_shape fields. Update kibana to enable fitting to bounds on geo_shape fields

@nreese nreese added release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.8.0 labels Apr 23, 2020
@nreese nreese requested a review from thomasneirynck April 23, 2020 12:50
@nreese nreese requested a review from a team as a code owner April 23, 2020 12:50
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 23, 2020

@elasticmachine merge upstream

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 27, 2020

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! :) 🚒

This is another case where the date-line issue is causing friction.

ES will return bounds that where the min_lon is larger than the max_lon when the coordinates are wrapped. The longitudes should be correctly rotated imho on the ES-side, but maybe Kibana can do a work-around.

The issue is here:

return {
minLon: esBounds.top_left.lon,
maxLon: esBounds.bottom_right.lon,
minLat: esBounds.bottom_right.lat,
maxLat: esBounds.top_left.lat,
};

Consider replacing with:

   let minLon = esBounds.top_left.lon;
    let maxLon = esBounds.bottom_right.lon;
    if (minLon > maxLon) {
      minLon -= 360;
    }

To reproduce the issue:

@thomasneirynck
Copy link
Copy Markdown
Contributor

I created elastic/elasticsearch#55812 to track ES-side

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 27, 2020

@elasticmachine merge upstream

@nreese nreese requested a review from thomasneirynck April 27, 2020 17:59
isFilterByMapBounds: () => {
return false;
},
it('should be false when buffers are the same', async () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff is really noisy for this change. Just removed source from updateDueToExtent signature

async getBounds(dataFilters) {
const isStaticLayer =
!this.getSource().isBoundsAware() || !this.getSource().isFilterByMapBounds();
const isStaticLayer = !this.getSource().isBoundsAware();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed isFilterByMapBounds check. This resulted in weird behavior where Elasticsearch layers would not call geo_bounds unless "dynamic filter" was selected

Copy link
Copy Markdown
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm when green.

Thx @talevy it's happening ;) !

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nreese nreese merged commit 1252b83 into elastic:master Apr 27, 2020
nreese added a commit to nreese/kibana that referenced this pull request Apr 27, 2020
* [Maps] geo_shape fit to bounds

* handle results that cross dateline

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
nreese added a commit that referenced this pull request Apr 27, 2020
* [Maps] geo_shape fit to bounds

* handle results that cross dateline

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:enhancement Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v7.8.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Maps] Cannot fit on layers with geo_shape documents

4 participants