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
osmdata package #103
Comments
One further comment prior to review: The |
Looking for reviewers. Will update thread when I do. |
Editor checks:
Editor commentsThank you for your submission! Out of curiosity, can you speak to how this relates to functionality from the Openstreetmap package in terms of overlap? While I seek reviewers I want to bring your attention to a few checks that were reuturned by
|
Thanks Karthik! In response to your Q, As for the goodpractice results:
|
FYI, while we don't strictly enforce line limits, putting |
Reviewer 1 is: @edzer |
Thanks @noamross, i didn't know about Thanks @karthik for arranging one reviewer so quickly, and thanks @edzer for agreeing to review. |
Okay, |
Reviewer 2: @ramnathv |
Package Review of osmdataPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
It would be good to mention in the first sentence of the pkg description that it returns vector data, and not images (like pkg OpenStreetMaps does): not everyone knows what's behind the Overpass API.
Paper (for packages co-submitting to JOSS)The package contains a
the target audience is not addressed in
Functionality
I didn't check for automated tests, or the guidelines. Final approval (post-review)
Estimated hours spent reviewing: 6-8 Review CommentsDesignThis package imports openstreetmap vector data in R. It has functionality similar to osmar, which has not seen any development since 2013; osmar can export to sp, osmdata to sp and sf. osmar, on the other hand, can do shortest paths through a network, and exposes the street network to R users (combining sp and igraph), osmdata does not expose the network. The package claims it is fast compared to sf/GDAL, but the link supporting this claim is dead. The package advertises itself mostly mentioning the overpass api, but not everyone familiar with R and (vaguely with) openstreetmap will know what that means; it would be good to also mention that osmdata reads openstreetmap vector data, as opposed to e.g. package OpenStreetMap which imports openstreetmap raster data (images). Some of the functions are rather verbose; e.g. function
which reduces 19 expressions to 1. Also, it is rather un R-like to make a class Function > x = list()
> x$bbox
# NULL this implies that The functions starting with
writes from doc into an object of class
writes the query to doc. Also, I had expected that option
The return of > hampi_sp
Object of class 'osmdata' with:
$bbox : 15.3159769,76.4406199,15.3559769,76.4806199
$overpass_call : The call submitted to the overpass API
$timestamp : [ Sun 1 Mar 2017 20:04:23 ]
$osm_points : 'sp' SpatialpointsDataFrame with 109 points
$osm_lines : 'sp' SpatiallinesDataFrame with 0 lines
$osm_polygons : 'sp' SpatialpolygonsDataFrame with 18 polygons
$osm_multilines : 'sp' SpatialmultilinesDataFrame with 0 multilines
$osm_multipolygons : 'sp' SpatialmultipolygonsDataFrame with 0 multipolygons where it might be better to use the class names of sp classes (SpatialpointsDataFrame etc. are not class names of sp, or of something else). Package sp does not distinguish between osm_lines and osm_multilines, or osm_polygons and osm_multipolygon; the list above confusingly suggests this. Why this distinction? Function namesNow, The vignette/README mention that
... but what is For the function names, it would help to memorize what they do if they would be, or contain, verbs; now, only
InstallDuring install, I see:
which points to a bug. The Performance, comparisonSince the the paper mentions data is "processed with very fast C++ routines for return to R", I compared the speed to that of the GDAL/OGR driver, using library(osmdata)
library(sf)
bb = c(11.561541, 48.14102, 11.582852, 48.1530)
q = opq(bbox = bb)
osmdata_xml(q, "x.osm")
system.time(o <- osmdata_sf(doc = "x.osm"))
layers = st_layers("x.osm")$name
osm_rd = function() { osmdata_sf(doc = "x.osm", quiet = TRUE) }
sf_rd = function() { system.time(lst <- lapply(layers, function(x) st_read("x.osm", x, quiet = TRUE))) }
library(microbenchmark)
microbenchmark::microbenchmark(osm_rd(), sf_rd(), times = 20)
# Unit: seconds
# expr min lq mean median uq max neval
# osm_rd() 2.393081 2.567723 2.645534 2.620662 2.708159 2.961327 20
# sf_rd() 2.948154 3.195789 3.351276 3.318939 3.479391 3.760206 20
and see that the results are quite similar. Of interest is the difference between the objects returned: o
# Object of class 'osmdata' with:
# $bbox : 11.3675909042358 48.0759544372559 11.7228775024414 48.2664070129395
# $overpass_call : The call submitted to the overpass API
# $timestamp : [ Sun 1 Mar 2017 20:02:02 ]
# $osm_points : 'sf' Simple Features Collection with 37997 points
# $osm_lines : 'sf' Simple Features Collection with 4251 linestrings
# $osm_polygons : 'sf' Simple Features Collection with 2682 polygons
# $osm_multilines : 'sf' Simple Features Collection with 248 multilinestrings
# $osm_multipolygons : 'sf' Simple Features Collection with 65 multipolygons
lst <- lapply(st_layers("x.osm")$name, function(x) st_read("x.osm", x, quiet = TRUE))
structure(sapply(lst, nrow), names = st_layers("x.osm")$name)
# points lines multilinestrings multipolygons
# 6445 4447 52 2427
# other_relations
# 192 since both functions read the same xml file, I was suprised to see that the number of features returned differs pretty much. I then looked a bit further into the GDAL OSM driver, which turns out to be highly configurable. Changing a few fields in the config file I got # points lines multilinestrings multipolygons
# 37997 4569 52 2427
# other_relations
# 192 Which at least gets the same number of points, and even more lines and polygons, than osmdata. The number of options did scare me somewhat, and made me wonder which osmdata.ini configuruation would correspond to the osmdata query result, and how these configuration options are handled by osmdata. By the query call? If so, do R users need to learn the OSM Overpass API query call language? Minor thingsThe link to sfa (vignette 2) is http://www.opengeospatial.org/standards/sfa, not http://www.opengeospatial.org/standards/sfo At several points, the vignettes misses the point that the word simple in simple features refers to "2D geometry with linear interpolation between vertices": simple (straight), as opposed to curved. ConcludingThis is a nice package that shows a significant programming effort to get OpenStreetMap vector data into R, using the R datastructures of either sp or sf. It does not give much directions what users can, or should do with these data. Compared to package The package claims to be considerably faster and to get more data, compared to the GDAL driver that is interfaced through e.g. R packages rgdal and sf. I could confirm neither of these claims: reading through I wonder, for an end-users, what the value is of getting a point set that consists of not only point features (such as traffic lights, or way signs) but also all vertices in lines and polygons. The vignettes of osmdata do not point out how, from an osmdata query result, one could for instance isolate the point features for plotting purposes, nor other user benefits of having the ID of every possible point available. |
Thanks @edzer for the useful comments and your detailed consideration of
Note that collective pronouns refer here to @mpadge, @Robinlovelace, and @maelle, unless otherwise explicitly indicated. easy to solve issues 😎First start with the easy bits 🍰, mostly already done with this commit:
open issues pending further discussion / debate 👷Now on to those points regarding which we are uncertain how best to respond. 🌰 Code
Package functionality
Comparison with
|
@ramnathv Gentle reminder that we hope to have your review in a couple of weeks (May 3rd) 🙏 |
@ramnathv Another quick ping. |
@karthik I need a few more days to wrap this up. Apologies for the delay. |
@ramnathv |
Hi @mpadge, even though we did not receive the second review, we are pleased with Edzer's and also note that the package has been successfully submitted to CRAN. So I am pleased to accept this package into the rOpenSci suite! 🎉 I've created a new team called Once you transfer, please add the rOpenSci footer badge at the bottom of your README.
Then you'll need to update CI links and badge links and also quickly reactivate CI services. We can help with this part. Please reach out with any questions. |
Thanks @karthik - i've requested the transfer and just have to presume you get it (github notifications not so good there - i just got told i didn't have permission to do that). And thanks especially for boldly going ahead while still technically waiting for rev#2. I'll update CI guff as soon as transferred, and will also do the required DOI archiving. Two questions in that regard:
|
@karthik All CI links and badges updated but I can't push becuase
|
Yes, that's sufficient. I'll handle your manuscript at the JOSS end.
Right before initiating the JOSS submission, let's make sure that the version you have submitted to CRAN is archived in Zenodo and has a DOI ready.
bummer. Can you give me (@karthik) admin access on your original repo and I will quickly make the move and give you admin here? |
@mpadge Please try pushing now. I was able to add your team with admin permission. |
@karthik slight teething probs with CRAN - as soon as |
@mpadge excellent. I'll follow up with next steps for JOSS. |
@mpadge Please submit your paper to JOSS and assign me as the editor on the submission screen please. http://joss.theoj.org/papers/new |
Sorry for slight delay @karthik - now submitted and good to go. |
Summary
Download OpenStreetMap (OSM) data and convert to native R objects defined by sf or sp packages. OSM datasets are extracted from the overpass API and processed with efficient C++ routines.
https://github.com/osmdatar/osmdata
Anyone interested in geospatial analysis and visualisation, especially geographers, urban and transport modellers and planners.
The package is similar to
GDAL
through the interface provided bysf
, yet produces fundamentally different results, as briefly described in the main vignette, and detailed at length in a second vignette.osmdata
also reproduces much functionality available in the CRAN packageosmar
, yet does so enormously faster (like at least thousands of times faster!) and with a more intuitive user interface. The package also imports data faster thansf
.osmar
only returns data defined by a bounding box, while theosmdata
interface to the overpass API allows highly customised and specific queries to be constructed (see examples in main vignette).osmdata
is also designed to complement theropensci
packageosmplotr
, and the latter package will be modified immediately following CRAN acceptance ofosmdata
.Requirements
Confirm each of the following by checking the box. This package:
Publication options
paper.md
with a high-level description in the package root or ininst/
.NOTE: As per the JOSS guidelines, the package will be submitted to a long-term repo subsequent to the review process
Detail
Does
R CMD check
(ordevtools::check()
) succeed? Paste and describe any errors or warnings:Does the package conform to rOpenSci packaging guidelines? Please describe any exceptions:
If this is a resubmission following rejection, please explain the change in circumstances:
If possible, please provide recommendations of reviewers - those with experience with similar packages and/or likely users of your package - and their GitHub user names:
R
packagessp
andsf
The text was updated successfully, but these errors were encountered: