Skip to content

Add OSM elevation to point POIs#619

Open
candux wants to merge 2 commits into
protomaps:mainfrom
candux:fix-peak-elevation
Open

Add OSM elevation to point POIs#619
candux wants to merge 2 commits into
protomaps:mainfrom
candux:fix-peak-elevation

Conversation

@candux

@candux candux commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Overview

I recently switched my personal project, https://wogibt.es/, from OpenMapTiles to Protomaps. Most attributes had a clear equivalent, but mountain peak elevation was missing: OSM ele values were not being exposed as the elevation
attribute on Protomaps POIs.

I do not think this is intended behavior, since natural=peak features commonly rely on ele for displaying summit elevation.

Example node: https://www.openstreetmap.org/node/1308250615/history/2

Before:

image

After:

image

Technical details

natural=peak features are usually OSM point features. The POI layer already copied the OSM ele tag to elevation, but only in the named-polygon pointOnSurface path. Point POIs therefore lost the attribute even when the source
feature had ele.

This change moves the elevation assignment into the common POI attribute chain, so OSM POIs can carry ele through to the output feature. Existing named-polygon behavior is preserved, and Planetiler still omits the attribute when
ele is absent.

A regression test was added for natural=peak with ele.

Verification

Ran:

  • PoisTest
  • full mvn package
  • spotless:check
  • git diff --check
  • before/after Saarland PMTiles scans

For the Saarland z15 scan:

  • Before: 0 / 489 peak features had elevation
  • After: 425 / 489 peak features had elevation

The remaining 64 peak features do not have OSM ele in the source data.

AI assistance

This change was prepared with help from OpenAI Codex 5.5 xhigh.

Natural peak features are usually OSM point features, but the POI layer only copied the OSM ele tag while building the named-polygon point-on-surface output. That meant point peaks were emitted without the elevation attribute even when the source feature had ele set.

Move the elevation assignment into the common POI attribute chain so all OSM POIs can carry ele through to the output feature. This preserves the existing named-polygon behavior and lets Planetiler omit the attribute naturally when ele is absent.

Add a regression test covering natural=peak with ele to make sure peak POIs expose elevation.

Verification: ran PoisTest, full mvn package, spotless:check, git diff --check, and before/after Saarland PMTiles scans. The Saarland z15 scan changed from 0/489 peak features with elevation before the fix to 425/489 after the fix; the remaining peaks lack OSM ele in the source.

AI assistance: this change was prepared with help from OpenAI Codex.
@bdon

bdon commented Jul 6, 2026

Copy link
Copy Markdown
Member

Thanks, this is a good candidate for inclusion: https://tilezen.readthedocs.io/en/latest/layers/#points-of-interest

I do think it makes more sense for the elevation to be a numerical type instead of a string, does that matter for your use case?

@candux

candux commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Numeric makes sense to me. It is semantically the better type, and the OSM wiki defines ele as elevation in metres, so exposing it as a number matches the source tag better than passing through the raw string.

One caveat: elevation already exists today in Protomaps for named polygon POIs as the raw OSM ele string. The Protomaps semantic versioning doc lists elevation as a common-optional property. It does not explicitly say how property type changes are handled, but changing a documented property from string to number could reasonably be considered a compatibility risk for users already consuming it as a string.

I did a quick check: OSM-wide, Taginfo shows ele values are overwhelmingly parseable as numbers, about 99.94% by object count. So the practical risk looks small, but not zero. A few non numeric values were

  • 4445'
  • 31 ft
  • 2 meter
  • 5m
  • 11;12
  • yes
  • fixme
  • ???

Given the OSM semantics and that Tilezen documents this as an elevation value, I’m happy to update the PR to parse ele into a numeric metre value and omit values that are not cleanly parseable. Would that small compatibility risk be acceptable to you?

@bdon

bdon commented Jul 7, 2026

Copy link
Copy Markdown
Member

I think elevation currently present as strings should be considered a bug and would prefer we consolidate on numerical values, even if it breaks existing consumers (I don't know of any right now and it is impossible to know for certain)

OSM ele represents elevation in metres, so expose POI elevation as a numeric tile value instead of passing through raw tag text. This applies both to polygon-derived POIs that already carried elevation and to point POIs added by the previous change.

Add a conservative parser that accepts clean signed decimal values, including an optional trailing m suffix with or without whitespace, and emits rounded integer metres. Omit elevation when ele contains other units or free text. Update POI tests to assert integer elevation for peak and named polygon output and cover decimal, metre-suffixed, and invalid values.

AI assistance: This change was prepared with help from OpenAI Codex.
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@candux

candux commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. I added the numeric conversion as an additional commit on the PR, consolidating elevation on a numeric integer value.

A float would also have been reasonable because OSM ele can contain decimal metre values. I went with integer metres because the OSM wiki describes ele as elevation in metres, and integer metre values are simpler to consume for most map/style use cases. Decimal metre values are rounded to the nearest integer.

The parser now accepts plain numeric OSM ele values and metre-suffixed values with or without whitespace, for example 569, 569.5, 569 m, and 569m. Non-metre/free-text values are omitted.

I did a test extract and everything looks as expected

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