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

vector_layers #42

Merged
merged 5 commits into from
Jul 30, 2018
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 51 additions & 2 deletions 3.0/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ A name describing the tileset. The name can contain any legal character. Impleme

## 3.11 `scheme`

OPTIONAL. Default: "xyz".
OPTIONAL. Default: "xyz".
Copy link
Contributor

Choose a reason for hiding this comment

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

This whitespace change should go in a different commit because it's unrelated.


Either "xyz" or "tms". Influences the y direction of the tile coordinates. The global-mercator (aka Spherical Mercator) profile is assumed.

Expand All @@ -141,7 +141,7 @@ A semver.org style version number. Describes the version of the TileJSON spec th

## 3.14 `tiles`

REQUIRED.
REQUIRED.
Copy link
Contributor

Choose a reason for hiding this comment

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

This whitespace change should go in a different commit because it's unrelated.


An array of tile endpoints. {z}, {x} and {y}, if present, are replaced with the corresponding integers. If multiple endpoints are specified, clients may use any combination of endpoints. All endpoints MUST return the same content for the same URL. The array MUST contain at least one endpoint. The tile extension is NOT limited to any particular format. Some of the more popular are: mvt, vector.pbf, png, webp, and jpg.

Expand All @@ -154,6 +154,55 @@ An array of tile endpoints. {z}, {x} and {y}, if present, are replaced with the
```

## 3.15 `vector_layers`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add subsections, defining each of the sub-items.


REQUIRED. (if describing vector tiles)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mentioned in #42 (comment) - if the tilejson spec isn't describing vector tiles, how is this field handled?

Perhaps it shouldn't be REQUIRED?


A JSON object whose value is an array of JSON objects. Each of those JSON objects describes one layer of vector tile data, and MUST contain the following key-value pairs:

* `id` (string): The layer ID, which is referred to as the `name` of the layer in the [Mapbox Vector Tile spec](https://github.com/mapbox/vector-tile-spec/tree/master/2.1#41-layers).
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear to me what the reference to the MVT spec is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per discussion in #14 it is to help connect what this value refers to according to the vector tile layer concept. The MVT spec is used as an example. If we remove this reference entirely, it could be confusing since it’s nice to point to how vector tile specifications refer to layer names. I think we can remove the reference in v4 once we rename the property to name.

Basically, if we're considering matching the id field to name to match the MVT specification, then the reference link to the MVT specification feels relevant.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be explicit that there is no requirement the id is unique, but some formats may prohibit overlapping id and zoom.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, the enforcement of this is vague.

cc @flippmoke @GretaCB

* `fields` (object): A JSON object whose keys and values are the names and descriptions of attributes available in this layer. Each value (description) MUST be a string. The values MAY describe the underlying data type, such as `String`, `Number`, or `Boolean`. Attributes whose type varies between features MAY be listed as `"String"`.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not really clear from this that the values are free-form text, and no special meaning should be given to an attempt to describe the underlying data type, since "Number" could be a description or a type, and different tile formats might have different allowable types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I think we should remove the concept of Number String Boolean entirely and just say this is string value that should describe the property. The explicit options are leftover from the mbtiles-spec, which is likely going to update once we ship v3.


Each layer object MAY also contain the following key-value pair:

* `description` (string): A human-readable description of the layer's contents.
* `minzoom` (number): The lowest zoom level whose tiles this layer appears in. MUST be greater than or equal to the tileset's `minzoom`.
* `maxzoom` (number): The highest zoom level whose tiles this layer appears in. MUST be less than or equal to the tileset's `maxzoom`.

These keys are used to describe the situation where different sets of vector layers appear in different zoom levels of the same tileset, for example in a case where a "minor roads" layer is only present at high zoom levels.

Implementations MUST treat unknown key-value pairs as if they weren't present. However, implementations MUST expose unknown key/values in their API so that API users can optionally handle these keys.
Copy link
Contributor Author

@mapsam mapsam May 8, 2018

Choose a reason for hiding this comment

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

@GretaCB I've noticed some implementations (mapbox streets tilejson for example) include other keys here. I'm thinking we can document that this is acceptable with the same language from the Structure section. Duplicating the text feels a little redundant, so I'm wondering if we could update the Structure introduction to say something like:

Implementations MUST treat unknown keys as if they weren't present. This is true at any depth of the JSON. However, implementations MUST expose unknown key/values in their API so that API users can optionally handle these keys.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this section is needed at all, given the text elsewhere in the spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mapsam what if we link to the Structure section to avoid needing to update text in both places, but also keep a reference in both places to avoid it being overlooked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me @GretaCB - will work an update!


```JSON
{
"vector_layers": [
{
"id": "roads",
"description": "Roads and their attributes",
"minzoom": 2,
"maxzoom": 16,
"fields": {
"type": "One of: trunk, primary, secondary",
"lanes": "Number",
"name": "String",
"sidewalks": "Boolean"
}
},
{
"id": "countries",
"description": "Admin 0 (country) boundaries",
"minzoom": 0,
"maxzoom": 16,
"fields": {
"iso": "ISO 3166-1 Alpha-2 code",
"name": "English name of the country",
"name_ar": "Arabic name of the country"
}
}
]
}
```


Copy link
Contributor

Choose a reason for hiding this comment

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

Stray newline

## 3.16 `version`

OPTIONAL. Default: "1.0.0".
Expand Down