-
-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Compute layer attr stats from actual vector tile features #752
Conversation
https://github.com/onthegomap/planetiler/actions/runs/7208618431 ℹ️ Base Logs d98d5d6
ℹ️ This Branch Logs 7135c84
|
@@ -19,7 +18,6 @@ message TileEntry { | |||
} | |||
|
|||
message InitializationEntry { | |||
Metadata metadata = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bbilger I need to remove metadata from the archive init call since it's not fully known until done writing all tiles. Any concerns about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no concerns 👍
just a note for the future: once 1.0 gets out, we might want to consider to ensure that changes in protobuf are backwards compatible - which in that case would mean to reserve the field number+name
but absolutely not needed now (pre 1.0), and I don't think anybody is using it, anyways
|
Looks like a useful fix to me! |
Tilejson stats in the archive metadata are currently computed while processing source features, but this misses vector tile attributes added and removed during post-processing. This PR moves computing the attr stats to tile-write time to capture the actual attributes written on features to the output archive.
A side effect of this is that tile archive
initialize
method can no longer accept metadata since the metadata isn't fully-known until the tiles are done being written so any archive implementation needs write metadata from thefinish
method.Fixes #749, see #746