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

[Non-schema Change] Add sql query files for mappings #258

Merged
merged 9 commits into from
Jul 29, 2024
Merged

Conversation

atiannicelli
Copy link
Contributor

@atiannicelli atiannicelli commented Jul 24, 2024

This adds the queries that map to the mapping that exist today. We have decided that SQL statements are better than the config yaml files that are there now so this converts those yaml files to the SQL files. These will be used by the building pipeline in the tf-data-platform repo once they are merged.

I also plan on removing the old config files once the tf-data-platform code has been switched over to use these new files.

I also fixed the subtype for bridge structures to be set to "transportation" if it is not filled in by other means.

Copy link
Contributor

@danabauer danabauer left a comment

Choose a reason for hiding this comment

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

just curious - does converting to lower case and trimming extra spaces greatly improve the mapping? Do we have other data cleaning mechanisms in place?

@jwass
Copy link
Collaborator

jwass commented Jul 29, 2024

I'm all for the element_at() change, but is the lower(trim(...)) really necessary? This feels like correcting OSM data that should instead be fixed at the source.

@atiannicelli
Copy link
Contributor Author

We do currently "correct/transform" the height values. We convert values with "m" "meters" "feet" "inches" and even ' and '' as units into a standardized height in meters.
I didn't think that the lower(trim( code would be considered changing the values as "Brick" is just as acceptable in OSM as "brick". It is not a big number that we will convert, but I just thought we don't want to go through OSM and start fixing capitalization issues.

By adding this code we will convert:
3,384 Building Materials (out os a total of 1,876,586 non null building:material or building:facade:material features)
6,514 Roof Materials (out of a total of 1,611,772 non null roof:material features)
5,551 Roof Shapes (out of a total of 6,874,732 non null roof:shape features)

Also, we currently use trim and lower in the names conversion. I thought it made sense to use it any place that we are comparing strings from OSM to a list of string values.

@jwass
Copy link
Collaborator

jwass commented Jul 29, 2024

Ah, right. I was actually thinking you were trimming or lowering that actual tags themselves, not the values. Yeah this makes total sense.

@jwass jwass self-requested a review July 29, 2024 14:11
@jwass jwass merged commit dcc35fb into dev Jul 29, 2024
2 checks passed
@jenningsanderson jenningsanderson deleted the building-mappings branch July 29, 2024 17:18
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.

4 participants