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

Supporting files for buildings theme #89

Merged
merged 11 commits into from
Feb 28, 2024

Conversation

LukaszMichalski-TomTom
Copy link
Contributor

@LukaszMichalski-TomTom LukaszMichalski-TomTom commented Dec 5, 2023

Feb 22, 2024: This PR has been resurrected and re-based, scroll to bottom for context


Added mapping files for OSM values to Overture enumeration values.
Additional values:
facadeMaterial:

  • clay
  • timber_framing

roofMaterial

  • gravel
  • solar_panels

roofShape:

  • saltbox
  • skillion
  • sawtooth

Four roof shape names changed to OSM equivalents, e.g. gablegabled. I did it to make it more in line with OSM tooling and possible renders that can generate roof geometry based on this value. If someone would like to use Overture data to generate visualization, it may be easier for them.

Copy link
Collaborator

@jwass jwass left a comment

Choose a reason for hiding this comment

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

@LukaszMichalski-TomTom I realize these aren't all your doing, but can you make all the Overture values camelCased?

It's the standard (even though I don't love it).

@jwass
Copy link
Collaborator

jwass commented Dec 7, 2023

@LukaszMichalski-TomTom Just so I understand... facadeMaterial describes the mapping for

  • building:material=* (and also building:facade:material?)
  • roofMaterial is for roof:material
  • roofShape is for roof:shape
    ??

@LukaszMichalski-TomTom
Copy link
Contributor Author

LukaszMichalski-TomTom commented Dec 12, 2023

@jwass

@LukaszMichalski-TomTom Just so I understand... facadeMaterial describes the mapping for

  • building:material=* (and also building:facade:material?)
  • roofMaterial is for roof:material
  • roofShape is for roof:shape
    ??

I will add README next to mapping files to describe osm tag that need to be used.
overture_facadeMaterial_OSM_mapping.json file contains mapping of OSM values to Overture facedMaterial property. OSM tags used for mapping in order:

overture_roofMaterial_OSM_mapping.json file contains mapping of OSM values to Overture roofMaterial property. OSM tags used for mapping:

overture_roofShape_OSM_mapping.json file contains mapping of OSM values to Overture roofShape property. OSM tags used for mapping in order:

@vcschapp
Copy link
Collaborator

Reminder: @jwass took the action to consult with the PR authors and determine if this can be brought back to life.

jwass
jwass previously approved these changes Feb 14, 2024
skmoore
skmoore previously approved these changes Feb 14, 2024
DavidKarlas
DavidKarlas previously approved these changes Feb 14, 2024
Copy link
Collaborator

@vcschapp vcschapp left a comment

Choose a reason for hiding this comment

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

Could you please:

  1. Fix facedMaterial to facadeMaterial? (typo in the README.md)
  2. Ensure the files end in a newline (should not have the no newline at EOF icon at the bottom of every diff)

task-force-docs/buildings/README.md Outdated Show resolved Hide resolved
@jenningsanderson jenningsanderson force-pushed the building-material-and-roofs-mappings branch from 91df02b to 75f724c Compare February 22, 2024 23:45
@jenningsanderson
Copy link
Collaborator

jenningsanderson commented Feb 23, 2024

@LukaszMichalski-TomTom I updated these to be snake_case in-line with the new schema updates and added them all to one file.

These are out-of-sync with the current logic in the buildings airflow pipeline and schema, which is causing hundreds of thousands of validation errors, which will soon cause the theme to fail promotion.

Can you confirm that these are the correct mappings (see the table below), then we can make sure that the pipeline and schema files are up-to-date?

@jenningsanderson
Copy link
Collaborator

Based on the current logic in the mapping JSON file (https://github.com/OvertureMaps/schema/pull/89/files#diff-5e18ae5feffd2d58657bf46be190bc986fb510fc7ee3b69f0e2d185828f4fad1), here are the counts for thee entire building theme:

roof_shape _count
gabled 4159376
flat 1370802
hipped 756957
pyramidal 249441
skillion 188343
half_hipped 100036
saltbox 55879
round 39348
gambrel 32996
mansard 23322
dome 16401
onion 5980
sawtooth 600
spherical 26
roof_material count
roof_tiles 856423
metal 187222
eternit 129954
concrete 127903
slate 58635
glass 38825
thatch 21904
grass 12637
plastic 7316
copper 7299
gravel 7153
wood 3715
solar_panels 179
facade_material count
brick 514852
cement_block 448061
plaster 398151
wood 210496
concrete 174449
metal 91201
stone 41789
glass 38035
clay 30661
plastic 16514
timber_framing 9975

@jenningsanderson
Copy link
Collaborator

I added tar_paper as an acceptable value because it's the 4th largest on OSM

@jenningsanderson jenningsanderson changed the title Added roof shape and material mapping files Supporting files for buildings theme Feb 27, 2024
Copy link
Collaborator

@vcschapp vcschapp left a comment

Choose a reason for hiding this comment

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

    ) )        /\
   =====      /  \
  _|___|_____/ __ \____________
 |::::::::::/ |  | \:::::::::::|
 |:::::::::/  ====  \::::::::::|
 |::::::::/__________\:::::::::|
 |_________|  ____  |__________|
  | ______ | / || \ | _______ |
  ||  |   || ====== ||   |   ||
  ||--+---|| |    | ||---+---||
  ||__|___|| |   o| ||___|___||
  |========| |____| |=========|
 (^^-^^^^^-|________|-^^^--^^^)
 (,, , ,, ,/________\,,,, ,, ,)
','',,,,' /__________\,,,',',;;

@jenningsanderson jenningsanderson merged commit 5ddb3a8 into dev Feb 28, 2024
2 checks passed
@vcschapp vcschapp deleted the building-material-and-roofs-mappings branch March 6, 2024 17:04
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.

8 participants