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

Improve oe_get_keys #257

Merged
merged 11 commits into from
May 18, 2022
Merged

Improve oe_get_keys #257

merged 11 commits into from
May 18, 2022

Conversation

agila5
Copy link
Contributor

@agila5 agila5 commented May 6, 2022

Fix #251. The result of the following commit is summarised below:

  1. Informative error when the selected layer does not exist:
library(osmextract)
#> Data (c) OpenStreetMap contributors, ODbL 1.0. https://www.openstreetmap.org/copyright.
#> Check the package website, https://docs.ropensci.org/osmextract/, for more details.
oe_get("ITS Leeds", download_only = TRUE, quiet = TRUE, download_directory = tempdir())
oe_get_keys("ITS Leeds", layer = "points", download_directory = tempdir())
#> Error: The matched file does not contain the selected layer. Check the examples in the docs to see how to add it.

oe_get("ITS Leeds", layer = "points", download_only = TRUE, quiet = TRUE, download_directory = tempdir())
oe_get_keys("ITS Leeds", layer = "points", download_directory = tempdir())
#>  [1] "amenity"                 "addr:postcode"          
#>  [3] "addr:street"             "addr:city"              
#>  [5] "fhrs:id"                 "capacity"               
#>  ...

Created on 2022-05-06 by the reprex package (v2.0.1)

  1. Warning when the keys were already extracted from the other-tags field
library(osmextract)
#> Data (c) OpenStreetMap contributors, ODbL 1.0. https://www.openstreetmap.org/copyright.
#> Check the package website, https://docs.ropensci.org/osmextract/, for more details.
oe_get(
  "ITS Leeds", download_only = TRUE, quiet = TRUE, 
  download_directory = tempdir(), extra_tags = c("foot", "lanes")
)
#> [1] "C:\\Users\\gilardi\\AppData\\Local\\Temp\\Rtmpi2s2FD\\test_its-example.gpkg"
oe_get_keys("ITS Leeds", download_directory = tempdir())
#>  [1] "surface"             "bicycle"             "lit"                
#>  [4] "access"              "oneway"              "maxspeed"           
#>  ... 

#> Warning: The following keys were already extracted from the other_tags field:
#> foot - lanes

Created on 2022-05-06 by the reprex package (v2.0.1)

I will write the tests after we merge #255

@agila5 agila5 requested a review from Robinlovelace May 6, 2022 12:57
@Robinlovelace
Copy link
Member

These look like really nice changes.

# )

# Therefore, I try to prefer the .gpkg files. The only problem is that
# some of the keys might be excluded from the other-tags field in a .gpkg
Copy link
Member

Choose a reason for hiding this comment

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

True that.


# Therefore, I try to prefer the .gpkg files. The only problem is that
# some of the keys might be excluded from the other-tags field in a .gpkg
# file, so I will add a warning message.
Copy link
Member

Choose a reason for hiding this comment

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

How about adding to the warning message to re-convert, e.g. with

  if (any(existing_fields %!in% default_fields)) {
    warning(
      "The following keys were already extracted from the other_tags field: ",
      paste0(setdiff(existing_fields, default_fields), collapse = " - "),
      "You can try again with force_vectortranslate = TRUE",
      call. = FALSE
    )
  }

if (layer %!in% sf::st_layers(zone)[["name"]]) {
stop(
"The matched file does not contain the selected layer. ",
"Check the examples in the docs to see how to add it.",
Copy link
Member

Choose a reason for hiding this comment

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

Worth adding this line below?

      "You can try again with force_vectortranslate = TRUE",

@agila5
Copy link
Contributor Author

agila5 commented May 6, 2022

Thanks, done!

@agila5 agila5 marked this pull request as ready for review May 18, 2022 17:44
@agila5 agila5 merged commit 73be3b1 into master May 18, 2022
@agila5 agila5 deleted the improve_oe_get_keys branch May 18, 2022 17:44
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.

[FEATURE] Improve oe_get_keys()
2 participants