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

Test on real data #46

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Test on real data #46

wants to merge 3 commits into from

Conversation

fnattino
Copy link
Contributor

Closes #24

I have also resized the packaged data, limiting the street network to the city boundary. The main reason is that in the outskirt of Bucharest I have found one motorway link that would give rise to a self-intersecting-yet-perfectly-valid stroke, but which would then be tricky to validate as implemented in the test (i.e. using sf::st_contains would not identify the constituting initial edges due to the self-intersection). Since this is some sort of an edge case which would only make the validation trickier without bringing more insight into the well-functioning of rcoins, I have decided for the easy fix i.e. to modify the dataset. As a derived benefit, the data script is also more concise (the "buffer" functionality was imported from CRiSp but not really needed).

@@ -51,24 +51,16 @@ get_osm_river <- function(river_name, bb, crs) {
river_centerline <- osmdata_as_sf("waterway", "river", bb)
river_centerline <- river_centerline$osm_multilines |>
filter(.data$name == river_name) |>
st_filter(sf::st_as_sfc(bb), .predicate = sf::st_intersects) |>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As also noticed in CRiSp, the Overpass API query for the river centerline now returns an additional branch of the Dambovita, not sure why, since it does not intersects the bounding box...

@fnattino fnattino requested a review from cforgaci January 15, 2025 13:59
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.

Add tests on real datasets
1 participant