Closed e-n-f closed 6 years ago
@incanus I incorporated most of these suggestions, except
Looks good. I will take a crack at the formatting, too.
To pretty-print the JSON within a Markdown bulleted list, try:
json
:
{
"vector_layers": [
{
"description": "Census counties",
"fields": {
"ALAND": "Number",
"AWATER": "Number",
"COUNTYFP": "String",
"GEOID": "String",
"MTFCC": "String",
"NAME": "String",
"NAMELSAD": "String",
"STATEFP": "String"
},
"id": "tl_2016_us_county",
"maxzoom": 5,
"minzoom": 0
},
{
"description": "Census primary roads",
"fields": {
"FULLNAME": "String",
"LINEARID": "String",
"MTFCC": "String",
"RTTYP": "String"
},
"id": "tl_2016_us_primaryroads",
"maxzoom": 5,
"minzoom": 0
}
]
}
Yeah, exactly. See here: https://gist.github.com/incanus/969d3cc9fd1f3a66263b33d90ff3ce68
Thanks. I guess that's clear enough that it's meant to be part of the bullet.
I'd rather abandon this in favour of 2.0.
@pnorman I agree that there's no need for this separate PR to exist if the 2.0 spec ends up containing all the necessary information to create working mbtiles files.
@ericfischer could you please also provide a diff to the previous version?
@kkaefer I don't know how to make github show a diff between two files with different names, but here it is as a gist: https://gist.github.com/ericfischer/121213249c8d87d632fb89579ba54672
It's best to do what I did and copy old_ver to new_ver in one commit then change new_ver so you can do a diff between the tip of the branch and the commit that copies. e.g. https://github.com/mapbox/mbtiles-spec/compare/14942c649e4cff48d3b0c393f856d7681890594a...pnorman:2.0
Great idea @pnorman. I'll make a new branch that does that.
Thanks again @pnorman. @kkaefer, the diff is https://github.com/mapbox/mbtiles-spec/compare/c689a6aaa2fb54835b36fb5c527b2a33b56fe081...vector-revision.
Thanks @kkaefer. I hope these latest revisions address your comments.
( language neutrality , multi-language support )
I added my opinion to https://github.com/mapbox/mbtiles-spec/pull/46#issuecomment-262257946 and probably also relevant for this spec.
Good point. I just added back the MIME type reference for these types.
Is mime type the defacto string for those?
I don't know. You're the one who said that those types were used in mbtiles in the wild. I have not seen those tilesets myself.
cc @flippmoke @sfkeller re: https://github.com/geometalab/Vector-Tiles-Reader-QGIS-Plugin/issues/112
From @pnorman in 2.0
pull request on things to be migrated here:
From what I see, vector_layers is mandatory but bounds (extent), minzoom and maxzoom are optional.
I'm aware of reasons to make them optional, but on the other hand, that's exactly the raison-d'etre having this metadata. Clients now have to scan the data to extract the required metadata by themselves which implies delays in order of seconds to minutes.
There are even situations where the extraction e.g. of bounds is impossible for a client: Look at the missing tiles approach at https://github.com/mapbox/tilelive-vector (to "avoid requiring many duplicate or empty vector tiles to be generated at high zoom levels, the backend source can specify a maskLevel..."). In other words, there are missing tiles on purpose (response is NULL/http 404) which triggers a lookup at client side of the tiles contents at maskLevel.
In addition when comparing with OGC WMS/WMTS, bounds are mandatory there too.
I therefore suggest to make at least bounds mandatory, if possible also minzoom and maxzoom.
@sfkeller I think I agree that it should be required in 1.3
.
That said, I do want to key in on something as we write this though, we need to be careful how we term optional and required in 1.3
. The intent of 1.3
is to "describe the existing world". I know that this might not always be the same between all implementations, but making things required that might have been optional in code or datasets could suddenly cause mbtiles files to fail processing where they might not have prior (if someone were to update to follow the spec in their code).
I think making this ideal should be shifted to 2.0
where I think we can be much more strict and we can have fallback approaches for old files still on the 1.x
. A much more strict difference in version might be nice? Its a very slight difference, but I think one we should consider as we move forward.
@flippmoke
The intent of 1.3 is to "describe the existing world"
I have checked the OSM QA TILES metadata
sqlite> select * from metadata;
scheme|tms
basename|hungary.mbtiles
id|31012018194303.planet
filesize|34391547904
name|hungary.mbtiles
description|hungary.mbtiles
version|2
minzoom|12
maxzoom|12
center|19.2041015625,47.171156436539384,12
bounds|16.171875,45.767522962149904,22.236328125,48.574789910928864
type|overlay
format|pbf
json|{"vector_layers":[{"id":"osm","description":"","minzoom":12,"maxzoom":12,"fields":{}}]}
and as I see - it has a following json
row - with empty fields
(object)
{
"vector_layers":[
{
"id":"osm",
"description":"",
"minzoom":12,
"maxzoom":12,
"fields":{}
}
]
}
imho: the fields
(object) is optional.
Is it possible to add an OSM QA Tiles
metadata as a second example?
@flippmoke
but making things required that might have been optional in code or datasets could suddenly cause mbtiles files to fail processing where they might not have prior (if someone were to update to follow the spec in their code).
Isn't that exactly the idea of a specification? The client then acts accordingly to 1.3
and discards the invalid mbtiles. IMHO the idea of a specification is to describe the current situation, but to say how clients have to behave. At the moment, it's exactly the other way round.
@mnboos frankly, that is the way it should be. However, development outpaced the specification and existing data in a format exists. I do not want to invalidate a large amount of existing users files if possible suddenly. This is Mapbox's fault. We should have updated the spec as needed.
@flippmoke Can you enumerate which existing (Mapbox) software and data would not be aligned with mandatory/required vector_layers, bounds (extent), minzoom and maxzoom?
@sfkeller -- just to clarify, I am all for making it required in this version (interested in @ericfischer 's view however). I just wanted to make sure it was clear about the intention of this release, so a flood of other review items would not be brought up as to how it should be written. This difference is why we decided to go with 1.3
first then make clear changes to how the future should be in 2.0
.
So you say that there exist large amounts of mbtiles user files that have no bounds, minzoom nor maxzoom? I'm astonished about this and somehow left in the dark which (Mapbox) software does not produce these metadata items.
Anyhow. How about this:
I hope that this processes won't be delayed again. And I suggest that a future spec.s won't be too ambitious since I fear, it could be then blocked by discussions about other far more fundamental additions.
So you say that there exist large amounts of mbtiles user files that have no bounds, minzoom nor maxzoom? I'm astonished about this and somehow left in the dark which (Mapbox) software does not produce these metadata items.
@sfkeller to be clear, I have no idea if it does or does not. If it does not we should fix this immediately I would think. I agree that we should make the language here to be required. I do not interact with this spec or its software almost at all. I am simply working to help facilitate this document to completion.
I hope that this processes won't be delayed again.
I hope not as well. I know that @ericfischer is on travel currently but will look into this on Monday when he is back. My goal is that Monday/Tuesday we can have this completed and merged.
And I suggest that a future spec.s won't be too ambitious since I fear, it could be then blocked by discussions about other far more fundamental additions.
I have no idea how ambitious the future spec should be here.
Thanks @ericfischer and @flippmoke and all for the release 1.3!
I've helped out for quite some time to standardize - actually since OGC & ISO 19000 started. But I've never seen such an immediate response like this after this release (see e.g. https://twitter.com/richardf/status/960802937860747264).
Now let's keep the pace and move 1. to sync TileJSON spec. (c.f. https://github.com/mapbox/tilejson-spec/issues/14#issuecomment-363370681) and 2. to MBTiles spec. 2.0.
@tmcw @willwhite @kkaefer @incanus
cc @migurski