Skip to content
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 support for 2x2 metatile #163

Merged
merged 6 commits into from
Mar 3, 2017
Merged

Add support for 2x2 metatile #163

merged 6 commits into from
Mar 3, 2017

Conversation

zerebubuth
Copy link
Member

This adds support for 2x2 metatile and lays the foundation for 512px tiles. It does this by making the tilequeue worker operate on metatile-sized work chunks, where that is identified by a lower zoom tile coordinate.

The database is queried for the spatial extent of the metatile, but with the "zoom level" parameters set to the nominal_zoom, which is a measure of display scale, rather than the zoom of the tile coordinate. The post-processing is also run at the nominal_zoom.

Following that, smaller tiles are cut from the metatile, and all of them are put into the zip file which is uploaded to S3. No extra processing is done, so all the feature selection is performed at the nominal_zoom in the database or post-processor. Tiles are formatted separately, so should not incur serialisation overheads or coordinate precision loss.

At the moment, this only really supports metatile.size=1 or 2, as larger sizes such as 4 would also make tiles for the intermediate zoom. Making this fully configurable is left for another task.

Note that setting metatile.size=2 in tileserver will probably also break it, since it expects to load and save single tiles at the moment. The next bit of work is to update both tileserver and tapalcatl to deal with different metatile sizes.

format=json_format, layer='all'),
dict(tile=json, coord=Coordinate(17, 123, 457),
dict(tile=json, coord=Coordinate(123, 457, 17),
Copy link
Member

Choose a reason for hiding this comment

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

I cringe every time I use this constructor too. You can make it more explicit which tends to make it more readable, at the expense of making it longer, eg:

Coordinate(column=457, row=123, zoom=17)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! I shall try to remember that next time. It will save me a lot of debugging!

else:
parent = _common_parent(parent, t)

return parent
Copy link
Member

Choose a reason for hiding this comment

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

This looks like just reduce(_common_parent, tiles) to me, but maybe we should leave it the way it is anyway because reduce is frowned upon?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't mind either - reduce is considerably shorter, and less likely to harbour bugs. However, is the loop based version more readable?

I know that functional stuff is not considered "Pythonic", so perhaps we should follow that guideline. @iandees, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

The code as-is is more readable/reviewable for me, so I would prefer to keep it like this. But as long as tests cover this function (and _common_parent()) I'm happy to change my mind.

else:
parent = _common_parent(parent, t)

return parent
Copy link
Member

Choose a reason for hiding this comment

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

The code as-is is more readable/reviewable for me, so I would prefer to keep it like this. But as long as tests cover this function (and _common_parent()) I'm happy to change my mind.

groups[key].append(tile)

metatiles = []
for group in groups.itervalues():
metatiles.extend(make_single_metatile(size, group, date_time))
parent = _parent_tile([t['coord'] for t in group])
Copy link
Member

Choose a reason for hiding this comment

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

Something to think about: with this syntax you're creating a generator (t['coord'] for t in group), creating a list by iterating over that generator ([]) and then passing the list into the _parent_tile() function.

Since the _parent_tile() function is simply iterating over the items, you don't need to generate the intermediary list and just pass in the generator:

>>> def foo(foos):
...   print type(foos)
...   for a in foos:
...     print a
...
>>> bar = [dict(a='p', b='c'), dict(a='d', b='e')]
>>> foo([b['a'] for b in bar])
<type 'list'>
p
d
>>> foo(b['a'] for b in bar)
<type 'generator'>
p
d

It's probably not worth the change here because the group is always going to be pretty small.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I didn't realise it was possible to do that, I thought that the enclosing [] around the generator were part of the required syntax for the list comprehension. Fixed in 90fceb5.

@zerebubuth zerebubuth merged commit a6b4e2b into master Mar 3, 2017
@zerebubuth zerebubuth deleted the zerebubuth/2x2-metatile branch March 3, 2017 15:34
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.

3 participants