[Minor] Division updates

[stepps00]

Category

What kind of change is this?
Please select one of the following four options.
Consult Pull request merging criteria for a description of each category.

1. Cosmetic change.
2. Documentation change by member.
3. Documentation change by Overture tech writer.
4. Material change.

Description

This pull request adds three new fields for use in the divisions theme - prominence, is_rendering, and is_processing. This also adds context to an existing column, class, which would be reworked to add more specific subtype detail.

class: This field would explicitly call out the type of locality (hamlet, city, village, etc.) being represented. As a v1, we'd essentially repurpose the local_type en value to create this enum. In the future, additional types could be defined and added, like 'megacity' for example.

prominence: Similar to a ranked hierarchy for localities, this field would rank the significance of a locality (Overture's opinion of the importance of a city) based on factors like population, capital status, and type. The use cases for this new field would include helping drive cartographic display.

The value of this field could be an integer, where a lower number indicates a high significance/importance.

is_processing: A boolean to indicate whether or not a feature geometry should be used for processing purposes. Maritime geometries and geometries used for processing would receive a 1 value.

is_rendering: A boolean to indicate whether or not a feature geometry should be used for rendering purposes. Land-clipped geometries and geometries used in map rendering would receive a 1 value.

I've also added definitions and examples for enum values and updated defs.yaml as needed. cc @DavidKarlas @jonahadkins, I'll review this with the schema task force and merge this once approved.

Reference

https://github.com/OvertureMaps/tf-admin/issues/82

Testing

Checklist

Checklist of tasks commonly-associated with schema pull requests. Please review the relevant checklists and ensure you do all the tasks that are required for the change you made.

1. Add relevant examples.
2. Add relevant counterexamples.
3. Update any counterexamples that became obsolete. For example, if a counterexample uses property A but is not intended to test property A's validity, and you made a schema change that invalidates property A in that counterexample, fix the counterexample to align it with your schema change.
4. Update in-schema documentation using plain English written in complete sentences, if an update is required.
5. Update Docusaurus documentation, if an update is required.
6. Review change with Overture technical writer to ensure any advanced documentation needs will be taken care of, unless the change is trivial and would not affect the documentation.

Documentation Website

Update the hyperlink below to put the pull request number in.

Docs preview for this PR.

[stepps00]

Some feedback on these new fields:

• If class is available, investigate using that field over locality_type
• Investigate if the locality_rank field would better fit in the set of cartographic properties
• Deliver these fields using value-neutral terms / not 'locality'-specific, if possible

[stepps00]

We discussed this change in the divisions task force meeting this morning, a few comments about these changes:

• Because these new fields may not be locality-specific in the future, I'm going to suggest prominence is used instead of locality_rank.

• Instead of locality_type, I'm going to suggest we re-purpose class to house these new locality type values and move the existing class values (land, potentially maritime) to new booleans, similar to how the base theme uses is_salt and is_intermittent booleans. This would result in a property structure like:

  • theme: divisions
  • type: division
  • subtype: locality
  • class: hamlet
  • is_land: 0
  • is_maritime: 1

If this works for the theme and doesn't muddle property usage, I can add a new commit to change the name of the locality_rank property and rework the class / new boolean properties.. cc @DavidKarlas @jonahadkins

[jonahadkins]

since the accepted hierarchy of theme>type>subtype>class exists across most themes, I agree it makes more sense to repurpose class for our needs. this will give us a lot of flexibility and future-proofing for working with class to define feature subtypes within divisions. given that, also seems like a good time to re-introduce is_maritime as a boolean, instead of mixing in land and maritime.

big ➕ 1️⃣ to prominence - i feel like that appropriately captures what were aiming for here without treading familiar cartographic ground (sort_key, rank, etc)

huge thanks @stepps00 for digging in deep here, lets ship some code!

[vcschapp]

since the accepted hierarchy of theme>type>subtype>class exists across most themes, I agree it makes more sense to repurpose class for our needs. this will give us a lot of flexibility and future-proofing for working with class to define feature subtypes within divisions. given that, also seems like a good time to re-introduce is_maritime as a boolean, instead of mixing in land and maritime.

big ➕ 1️⃣ to prominence - i feel like that appropriately captures what were aiming for here without treading familiar cartographic ground (sort_key, rank, etc)

huge thanks @stepps00 for digging in deep here, lets ship some code!

I'm in a agreement with Jonah, and love the prominence idea. Can we have it as cartography.prominence?

[vcschapp]

We discussed this change in the divisions task force meeting this morning, a few comments about these changes:

• Because these new fields may not be locality-specific in the future, I'm going to suggest prominence is used instead of locality_rank.

• Instead of locality_type, I'm going to suggest we re-purpose class to house these new locality type values and move the existing class values (land, potentially maritime) to new booleans, similar to how the base theme uses is_salt and is_intermittent booleans. This would result in a property structure like:

  • theme: divisions
  • type: division
  • subtype: locality
  • class: hamlet
  • is_land: 0
  • is_maritime: 1

If this works for the theme and doesn't muddle property usage, I can add a new commit to change the name of the locality_rank property and rework the class / new boolean properties.. cc @DavidKarlas @jonahadkins

@stepps00 do it seems you are suggesting to repurpose class for areas and boundaries? Do we need to do that just to get class into division features?

[stepps00]

do it seems you are suggesting to repurpose class for areas and boundaries? Do we need to do that just to get class into division features?

That makes the most sense to me. If we leave the class values as-is, we'd end up with:

• "land" as the class value for division_area features
• "land" as the class value for division_boundary features
• "city"/"hamlet"/"megacity" etc. as the class values for division features

The better approach seems to be flagging maritime and land values separately from any "place" types. But @vcschapp curious what you think here.

[stepps00]

The divisions task force discussed this pull request this morning; here is a summary of the changes being suggested:

• A new is_processing column (rather than is_maritime to indicate whether or not a feature geometry should be used for processing purposes.
• A new is_rendering column (rather than is_land to indicate whether or not a feature geometry should be used for rendering purposes.
• A new prominence column to indicate the significance / importance of a place.
• A reworked/added class column to give more detail to a specific administrative subtype (hamlet, village, town, city, etc)

I've also added definitions and examples for enum values and updated defs.yaml as needed. cc @DavidKarlas @jonahadkins, I'll review this with the schema task force and merge this once approved.

[vcschapp]

@stepps00 can we decouple the 3 sets of changes into 3 PRs?

The change-groups I see are:

1. class changes to support hamlet, etc. This is for features of type division.
2. prominence change which would presumably become part of the cartography object?
3. The to add those is_* booleans...

The reason I'm suggesting decoupling is I think each can be applied independently and its easier then to get smaller units of change approved, since one thing that doesn't work won't block two that does.

[vcschapp]

• Documentation: Booleans are true or false in JSON schema, not 0/1!
• Need type.
• I think these are meant to be on division area/boundary?
• We should push ourselves a bit harder on the naming.