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

Feature/transit stops osm #72

Merged
merged 9 commits into from
Sep 11, 2024
Merged

Feature/transit stops osm #72

merged 9 commits into from
Sep 11, 2024

Conversation

weiqi-tori
Copy link
Member

@weiqi-tori weiqi-tori requested review from chrowe and tedw0ng September 4, 2024 05:05
tedw0ng
tedw0ng previously requested changes Sep 4, 2024
Copy link
Member

@tedw0ng tedw0ng left a comment

Choose a reason for hiding this comment

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

Please also add these values to 'railway': 'subway_entrance', 'station'

@weiqi-tori
Copy link
Member Author

Please also add these values to 'railway': 'subway_entrance', 'station'

Gotcha, added

Copy link
Contributor

@kcartier-wri kcartier-wri left a comment

Choose a reason for hiding this comment

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

If possible, it may be worthwhile adding a docstring to the OpenStreetMapClass that describes the high-level criteria for including the specific OSM features in the class. What is the underlying goal. Is it simply that the Data Scientists have requested these specific features or are we trying to produce a “base map” of some sort toward some goal? This would allow us to determine if the set is complete.

For example, given that wetlands are a critical environmental landform, why are we not currently specifying ‘natural’:[wetland’] under water?

Is there an internal page that describes how we decide which features to include and why?

It may also be useful to include a pointer to the OSM features page Map features - OpenStreetMap Wiki

Thanks Tori.

@S-AI-F
Copy link
Member

S-AI-F commented Sep 10, 2024

The OSM features are used for calculating the indicators that are served in the dashboard and not for basemap (fir this case). For exemple, 'open space' and 'health sites' are used for calculating proximity to open space areas or health service. The indicators methods are defined in the technical note used for this work. Agree that we might probably have documentation or links to the published technical in the code base.

@S-AI-F
Copy link
Member

S-AI-F commented Sep 10, 2024

The tech note describinh the indicator methids and how the layers are defined: https://www.wri.org/research/calculating-indicators-global-geospatial-datasets-urban-environment

@S-AI-F
Copy link
Member

S-AI-F commented Sep 11, 2024

We are adding a new ticket for documenting the layers classes and selected methods

Copy link
Contributor

@chrowe chrowe left a comment

Choose a reason for hiding this comment

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

Looks good to me

@chrowe chrowe dismissed tedw0ng’s stale review September 11, 2024 15:11

Requested changes were made

@chrowe chrowe merged commit bb431cb into main Sep 11, 2024
1 check passed
@chrowe chrowe deleted the feature/transit_stops_osm branch September 11, 2024 15:11
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.

5 participants