Skip to content

Add bing_tile_children and bing_tile_parent functions#14896

Merged
arhimondr merged 1 commit intoprestodb:masterfrom
jagill:tile-children
Jul 30, 2020
Merged

Add bing_tile_children and bing_tile_parent functions#14896
arhimondr merged 1 commit intoprestodb:masterfrom
jagill:tile-children

Conversation

@jagill
Copy link
Contributor

@jagill jagill commented Jul 26, 2020

Small helper functions to find the children and parent of a BingTile.

== RELEASE NOTES ==

Geospatial Changes
* Introduce ``bing_tile_children`` and ``bing_tile_parent`` functions to get parents and children of a Bing tile.

@jagill jagill requested a review from mbasmanova July 26, 2020 15:22
@jagill jagill changed the title Add BingTile.findChildren() and findParent() Add bing_tile_children and bing_tile_parent functions Jul 26, 2020
@jagill jagill requested a review from tdcmeehan July 27, 2020 12:31
Copy link
Member

@arhimondr arhimondr left a comment

Choose a reason for hiding this comment

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

LGTM % nits

Copy link
Member

Choose a reason for hiding this comment

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

please add a message in case it ever fails, e.g.: checkArgument(newZoom <= MAX_ZOOM_LEVEL, "newZoom must be less than or equal to %s: %s", MAX_ZOOM_LEVEL, newZoom), same for other checks

Copy link
Member

Choose a reason for hiding this comment

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

Add the original exception as a cause. Propagate original message. Same for other methods

Copy link
Member

Choose a reason for hiding this comment

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

Return children ...

Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member

Choose a reason for hiding this comment

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

assertThatThrownBy(() -> { ... }).hasMessage(...);

Same for other exception checks.

You can also check the exception type and exception fields. Generally the assertThatThrownBy construct is very powerful

Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe wrap, e.g.:

tiles.stream()
  .map(BingTile::toQuadKey)
  .sorted()
  .collect(Collectors.toList())

Also toImmutableList (statically imported)

Small helper functions to find the children and parent of a BingTile.
@arhimondr arhimondr merged commit 53a26bb into prestodb:master Jul 30, 2020
@jagill jagill deleted the tile-children branch July 30, 2020 19:18
@caithagoras caithagoras mentioned this pull request Aug 14, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants