Replace jdx

[rafapereirabr]

Message from CRAN:

This package [r5r] imports jdx, whose maintainer has requested it be archived (it does
not work with current Java 17).

We will defer this to Oct 27, at which point r5r will also need to be
archived if it still makes use of jdx.

[rafapereirabr]

This sets a hard deadline for our next update. Ping to @ansoncfit in case there are some contributions/changes to documentation Anson would like to suggest before this coming update of r5r.

[rafapereirabr]

Hi @mvpsaraiva! I've run the usual tests and it all seemed Ok.

devtools::check(pkg = ".",  cran = FALSE, env_vars = c(NOT_CRAN = "true"))

However, something strange popped up when I tried to replicate the results of our paper. The accessibility result we get in Figure 3 looks quite different from the original one. See below the plot I'm getting:

w:

[mvpsaraiva]

That's odd... I've just knitted the whole paper and the result seems fine:

I'll do some extra tests to make sure we get correct results from all functions before we merge this into the master branch and update the package on CRAN.

Obs: I had to fix some na.omit() in the paper's code to get the same picture that was published, as we must have changed it some time.

[mvpsaraiva]

I've made some tests comparing the results of r5r's public functions (travel_time_matrix, detailed_itineraries, accessibility, transit_network_to_sf, street_network_to_sf, find_snap) with and without jdx. All results are exactly the same. The code is in this repository.

We also had some performance improvements:

[rafapereirabr]

Hi @mvpsaraiva . The test scripts you've created work like a charm! I've run the tests on my local Windows machine and the outputs are identical. The only minor difference is that the performance of detailed_itineraries() looks a tiny bit slower with java_to_dt on Windows (see figure below). Overall, I would say we are ready to merge the dev branch into master and say goodbye to jdx dependency.

[rafapereirabr]

The line 34 of of the java_to_dt() function is currently this:

dt <- data.table::as.data.table(dt)

I've replaced it with setDT() in f138edb. setDT() is more memory efficient and it can make a big difference for large outputs. All tests have passed, but please me know if this creates any problem.
data.table::setDT(dt)

[mvpsaraiva]

I've replaced it with setDT() in f138edb. setDT() is more memory efficient and it can make a big difference for large outputs. All tests have passed, but please me know if this creates any problem.

Great! I was going to ask to the data.tablers to check if I was doing things the best way :)
I didn't know we could use setDT() directly into a list of vectors.

I'll run the test scripts again, just to make sure everything is fine.

[mvpsaraiva]

Everything is fine! I've merged the dev branch into the master now.

[rafapereirabr]

Amazing work, Marcus ! I give you the pleasure to close this issue :)

[mvpsaraiva]

Amazing work, Marcus ! I give you the pleasure to close this issue :)

Thanks, sir!