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

basic outdoor pois #706

Merged
merged 4 commits into from
Apr 14, 2016
Merged

basic outdoor pois #706

merged 4 commits into from
Apr 14, 2016

Conversation

nvkelso
Copy link
Member

@nvkelso nvkelso commented Apr 13, 2016

Connects to #662.

  • add basic outdoor pois
  • update migration to include basic outdoor pois
  • [tests] add tests for basic outdoor pois
  • [docs] update poi layer kind values

@nvkelso nvkelso added this to the v0.10.0 milestone Apr 13, 2016
@nvkelso
Copy link
Member Author

nvkelso commented Apr 13, 2016

Tests pass. Hacked Bubble Wrap to show a few of them (they need icon artwork, not these generic dots!).

screen shot 2016-04-12 at 17 07 00

screen shot 2016-04-12 at 16 59 56

screen shot 2016-04-12 at 16 55 58

@@ -414,3 +414,45 @@ filters:
min_zoom: 15
output:
kind: {col: tags->whitewater}
- filter: {aerialway: pylon}
Copy link
Member

Choose a reason for hiding this comment

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

If we're adding a utilities layer, would this be more appropriate to add to that instead of pois?

Copy link
Member

Choose a reason for hiding this comment

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

And perhaps power_pole and power_tower too?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, no, I got confused about the pylon - that's not for electricity but for holding up aerialway cables. Still, is pois the appropriate place for this? I am sure that many people are interested in pylons, but perhaps it's not traditionally considered a point of interest.

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 question! We have existing features like traffic lights, waste bins, & etc in the pois layer now (for historic reasons). Would those also move into a utilities layer? Maybe it should be called something more generic like infrastructure instead (and also move the aerialway=pylon)? Would barrier lines move from the existing boundaries layer to a new infrastructure layer? Would most amenity and shop values go into the pois layer, but many of the man_made values go into infrastructure instead?

Moving features into a dedicated infrastructure layer would mean there's less chance of them getting displayed mixed in with regular business pois, but it also increases the complexity of where to find things to build the map style. We had been discussing centralizing even more "poi" like features into the pois layer – we do this now for shops and other tags present on buildings, we peal those off and put them into the pois layer leaving only (icon-less) label placements behind. But in the landuse layer label placements should generally have icons so we already mix this up a bit.

Where would features like amenity: bbq and amenity: life_ring and amenity: picnic_table go? Seems like most of these features could just go into an infrastructure layer just as well as a pois layer. Sure they have an amenity tag from OSM, but looking at it not knowing the history it's not clear. Less layers seems better.

Copy link
Member

Choose a reason for hiding this comment

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

It seems like we need some criteria about which layer something goes into? Otherwise, we might as well just have points, lines and polygons layers. Why is, for example, landuse separate from buildings or earth, and are there similar categories to break down pois into?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's split off discussion of where the new power_pole and power_tower should go into #715 and move forward with merging this PR.

@zerebubuth
Copy link
Member

👍 merge!

@nvkelso nvkelso merged commit a8e6220 into master Apr 14, 2016
@nvkelso nvkelso deleted the nvkelso/basic-outdoor-pois branch April 14, 2016 15:00
@nvkelso nvkelso removed the in review label Apr 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants