Submission: opentripplanner

[mem48]

Submitting Author: Malcolm Morgan (@mem48)
Repository: https://github.com/ITSLeeds/opentripplanner
Version submitted: 0.1.0
Editor: @geanders
Reviewer 1: TBD
Reviewer 2: TBD
Archive: TBD
Version accepted: TBD

---

• Paste the full DESCRIPTION file inside a code block below:

Package: opentripplanner
Title: OpenTripPlanner for R
Version: 0.1.0.0000
Authors@R: c(
    person("Marcus", "Young",   email = "M.A.Young@soton.ac.uk", role = c("aut"),
      comment = c(ORCID = "0000-0003-4627-1116")),
    person("Malcolm", "Morgan", email = "m.morgan1@leeds.ac.uk", role = c("aut","cre"), 
      comment = c(ORCID = "0000-0002-9488-9183")),
    person("Robin", "Lovelace", email = "rob00x@gmail.com",      role = "aut",
      comment = c(ORCID = "0000-0001-5679-6536")),
    person("Layik", "Hama",     email = "layik.hama@gmail.com",  role = "ctb",
      comment = c(ORCID = "0000-0003-1912-4890"))
    )
Maintainer: Malcolm Morgan <m.morgan1@leeds.ac.uk>
Description: An R interface for OpenTripPlanner <http://www.opentripplanner.org/>.
    OpenTripPlanner (OTP) is an open source platform for multi-modal and multi-agency
    journey planning written in Java. OTP provides a REST API which OpenTripPlanner
    for R uses to connect to OTP.
Language: EN-GB
License: file LICENSE
URL: https://github.com/ITSLeeds/opentripplanner, https://itsleeds.github.io/opentripplanner/
BugReports: https://github.com/ITSLeeds/opentripplanner/issues
Encoding: UTF-8
LazyData: true
Imports:
    checkmate,
    httr,
    sf,
    googlePolylines,
    dplyr,
    geodist,
    jsonlite,
    pbapply
RoxygenNote: 6.1.1
Suggests: 
    testthat,
    knitr,
    rmarkdown,
    covr
VignetteBuilder: knitr

Scope

• Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

  • data retrieval
  • data extraction
  • database access
  • data munging
  • data deposition
  • reproducibility
  • geospatial data
  • text analysis

• Explain how and why the package falls under these categories (briefly, 1-2 sentences):
  Open Trip Planner is a multimodal route planning tool written in Java, the package allows R users to access the functionality of Open Trip Planner running locally or on a server. So, the package performs the role of data retrieval of geospatial data.

• Who is the target audience and what are scientific applications of this package?
  Transport planners and people engaging in transport-related research. Route planning is the basis of many different transport related research questions such as traffic management, accessibility measurement, and infrastructure planning. The package is being actively developed to support research at the University of Leeds Institute for Transport Studies.

• Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
  There are many packages that enable route planning in R (e.g. osrmr & dodgr) However there are no packages on CRAN that connect R to the Open Trip Planner. We are aware of one package on GitHub (https://github.com/datasciencecampus/access-to-services/tree/develop/propeR ) which relates to connecting R and OTP although its purpose is more specialised.
  A key benefit of OTP over other route planning tools is its integration of public transport and other modes of travel (e.g. bike hire) that are not supported by other tools such as osrm.

• If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Technical checks

Confirm each of the following by checking the box. This package:

• does not violate the Terms of Service of any service it interacts with.
• has a CRAN and OSI accepted license.
• contains a README with instructions for installing the development version.
• includes documentation with examples for all functions.
• contains a vignette with examples of its essential functions and uses.
• has a test suite.
• has continuous integration, including reporting of test coverage using services such as Travis CI, Coveralls and/or CodeCov.

Publication options

• Do you intend for this package to go on CRAN?
• Do you wish to automatically submit to the Journal of Open Source Software? If so:

JOSS Options

• The package has an obvious research application according to JOSS's definition.
  • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
  • The package is deposited in a long-term repository with the DOI:
  • (Do not submit your package separately to JOSS)

• Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

MEE Options

• The package is novel and will be of interest to the broad readership of the journal.
• The manuscript describing the package is no longer than 3000 words.
• You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
• (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
• (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
• (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

• I agree to abide by rOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

[mem48]

Submission after #291

[melvidoni]

Hello @mem48! Thanks for the submission. @geanders has been assigned as your editor.

[mem48]

Thanks, @melvidoni and @geanders looking forward to your feedback.

[geanders]

@mem48 : So sorry for the delay in the initial checks from my end! This looks like a very exciting package, and I'm looking forward to managing the submission. I'll start with some results and comments from some standard pre-review checks.

Editor checks:

• Fit: The package meets criteria for fit and overlap
• Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
• License: The package has a CRAN or OSI accepted license
• Repository: The repository link resolves correctly
• Archive (JOSS only, may be post-review): The repository DOI resolves correctly
• Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

---

Editor comments

From spelling::spell_check_package:

All seem fine. There don't seem to be any legitimate typos caught by spell checking.

From unit tests:

First, as a note to reviewers, you'll need a system variable named "I_have_OTP" with the value "TRUE" if you want to run all the test (otherwise, a number are skipped). You can create this from R with the call:

Sys.setenv(I_have_OTP = "TRUE")

(@mem48 : You may want to consider changing this to use a "NOT_CRAN" system variable instead, as this seems to be used more frequently for these types of checks of whether you're using a local system or not.)

I am consistently getting an error for one of the tests:

test_03_with_OTP.R:186: failure: otp_stop
grepl("SUCCESS", foo) isn't true.

Do you have any thoughts on whether this is a true error, or if I've neglected something needed for the package in setting up my system? I'm thinking likely the latter, as your Travis CI build is passing. In either case, any tips on resolving this on my system would be helpful, as I need to resolve this error to run a cleaner test of the testing coverage of the package.

From goodpractice::gp():

It is good practice to

  ✖ avoid long code lines, it is bad for readability. Also, many people prefer editor windows that are about 80
    characters wide. Try make your lines shorter than 80 characters

    R/otp-config.R:84:1
    R/otp-config.R:85:1
    R/otp-config.R:89:1
    R/otp-config.R:90:1
    R/otp-config.R:91:1
    ... and 131 more lines

  ✖ avoid sapply(), it is not type safe. It might return a vector, or a list, depending on the input data.
    Consider using vapply() instead.

    R/otp-plan.R:214:20
    R/otp-plan.R:230:13

  ✖ avoid the library() and require() functions, they change the global search path. If you need to use other
    packages, import them. If you need to load them explicitly, then consider loadNamespace() instead, or as a last
    resort, declare them as 'Depends' dependencies.

    R/otp-plan.R:165:7

  ✖ checking tests ... ERROR Running the tests in ‘tests/testthat.R’ failed. Last 13 lines of output: 6188 ??
    0:00.00 grep java ── 1. Failure: otp_stop (@test_03_with_OTP.R#186) 
    ────────────────────────────────────────────────────────────────────────────────────── grepl("SUCCESS", foo)
    isn't true. ══ testthat results
    ════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
    OK: 92 SKIPPED: 0 FAILED: 1 1. Failure: otp_stop (@test_03_with_OTP.R#186) Error: testthat unit tests failed In
    addition: Warning messages: 1: In getClassDef(class1) : closing unused connection 4 (<-localhost:11403) 2: In
    getClassDef(class1) : closing unused connection 3 (<-localhost:11403) Execution halted
  ✖ avoid 'T' and 'F', as they are just variables which are set to the logicals 'TRUE' and 'FALSE' by default,
    but are not reserved words and hence can be overwritten by the user.  Hence, one should always use 'TRUE' and 'FALSE'
    for the logicals.

    R/otp-plan.R:NA:NA

Based on these outputs, could you please:

• Shorten code lines longer than 80 characters
• Change uses of sapply to vapply or another safer alternative if possible
• Take out the use of library or require in the "opt-plan.R" file if possible (or explain in this thread why it needs to be kept---I see it's calling the current package, opentripplanner, for a parallel implementation, so maybe it's hard to work around using library for that?)
• Fix the "otp_stop" unit test (if it's a general problem, not just something I've missed in the set-up for my system)
• Change use of "T" and "F" to "TRUE" and "FALSE"

Documentation

Really nice job on the extensive online documentation for this package! I had a few comments for you to consider on the documentation at this stage

• You refer to "GTFS" a few times, but I didn't see it defined. Will this acronym be clear to most of your readers? If not, could you define it in the documentation?
• Is the geographical coverage of OpenTripPlanner worldwide? If so, could you specify that in the "getting started" document, or if not, clarify the geographical coverage somewhere?
• For the "prerequisites", I think your link to download Java if someone doesn't already have it might need to be fixed. I downloaded Java for my system from this link and installed it, but then got a message when I tried to run the package code that I needed "JDK", so I ended up installing that directly from Oracle's Java page and then everything ran successfully. You may want to doublecheck where you send people to download Java (and maybe specify that they'll need JDK, if that is indeed the case).

You can go ahead and work on these if you'd like, but none are egregious (the testing issue would be the most time-sensitive to get done), so I'm comfortable starting to look for reviewers for the package now. I'll update this comment with the names of the reviewers and their due date once I find them. Please let me know throughout the process if you have any questions!

---

Reviewers:
Reviewer #1: @kimnewzealand
Reviewer #2: @mifioriti

Due date: July 2, 2019

[mem48]

@geanders Thanks for the feedback. I'm trying to reproduce the error you are getting with the tests. Otherwise, I think these are all easy fixes.

[geanders]

@mem48 : That sounds good. If you are unable to reproduce the test error I got, then I think we'd be okay with seeing if it comes up for the reviewers.

[Robinlovelace]

Just looking at this. Please could you provide info on your system @geanders ? Suspect it may be an OS-specific issue. I'm on Ubuntu and also fails. otp_stop() is interactive, and asks:

otp_stop()
This will force Java to close, Press [enter] to continue, [escape] to abort
The following Java instances have been found:
 3659 pts/0    00:00:19 java

[geanders]

That seems plausible, @Robinlovelace. I'm using MacOS Mojave 10.14.4.

[Robinlovelace]

Think I've fixed that issue now.