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

Add return_pbf and return_gpkg arguments #253

Merged
merged 4 commits into from May 3, 2022
Merged

Add return_pbf and return_gpkg arguments #253

merged 4 commits into from May 3, 2022

Conversation

agila5
Copy link
Collaborator

@agila5 agila5 commented May 2, 2022

At the moment, the function oe_find() may return pbf or gpkg files without any particular constraint. This PR adds two new arguments named return_pbf and return_gpkg to select which format should be returned/excluded.

@agila5 agila5 requested a review from Robinlovelace May 2, 2022 16:04
Copy link
Member

@Robinlovelace Robinlovelace left a comment

Choose a reason for hiding this comment

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

Approving as can see this is an improvement but I suggest merging after responding to my comments. Great work Andrea!

@@ -1,33 +1,42 @@
#' Get the location of files
#' Get the path of .pbf and .gpkg files associated with an input OSM extract
Copy link
Member

Choose a reason for hiding this comment

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

👍

R/find.R Outdated
#' `place = "Isle of Wight"`, then the input `place` is matched with a URL of
#' a `.osm.pbf` file (via [`oe_match()`]) and the matching is performed setting a
#' pattern equal to the basename of that URL.
#' performed using `list.files()`, setting the `pattern` equal to the basename
Copy link
Member

Choose a reason for hiding this comment

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

#'   performed using `list.files()`, setting the `pattern` argument equal to the basename

Would be even more specific, I suggest that.

#' If there is no file in `download_directory` that can be matched with the
#' basename and `download_if_missing` parameter is equal to `TRUE`, then the
#' function tries to download and translate a new file from the chosen
#' If there is no file in the `download_directory` that can be matched with the
Copy link
Member

Choose a reason for hiding this comment

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

👍

R/find.R Outdated
#' @param ... Extra arguments that are passed to [`oe_match()`] and [`oe_get()`].
#' Please note that you cannot modify the argument `download_only`.
#' @inheritParams oe_get
#'
#' @return A character vector of length one (or two) representing the path(s) of the
#' corresponding `.pbf` (and `.gpkg`) files.
#' `.pbf`/`.gpkg` files associated with the input `place`.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe say what happens when both .pbf and .gpkg files are returned. Is the .pbf file path listed first? I assume so. Worth mentioning that if so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually no since the files returned by list.files are listed in alphabetical order (and .gpkg < .osm.pbf). I will add a note, thanks.

@agila5
Copy link
Collaborator Author

agila5 commented May 3, 2022

Thanks for the review @Robinlovelace, will merge now

@agila5 agila5 merged commit 07339fd into master May 3, 2022
@agila5 agila5 deleted the improve-oe-find branch May 3, 2022 09:25
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.

None yet

2 participants