ropensci / osmdata

R package for downloading OpenStreetMap data
https://docs.ropensci.org/osmdata
317 stars 45 forks source link

Release osmdata 0.2 #313

Closed mpadge closed 1 year ago

mpadge commented 1 year ago

Prepare for release:

Submit to CRAN:

Wait for CRAN...


Note from win-builder checks:

Package CITATION file contains call(s) to old-style personList() or as.personList(). Please use c() on person objects instead. Package CITATION file contains call(s) to old-style citEntry(). Please use bibentry() instead.

mpadge commented 1 year ago

Nope, last commit to CITATION still didn't fix it. Now CRAN winbuilder says:

Package CITATION file contains call(s) to old-style citEntry(). Please use bibentry() instead.

mpadge commented 1 year ago

First attempt got rejected because of SystemRequirements: C++11. Ways around this have recently been discussed on r-pkg-devel list. That thread concluded with this:

It turns out that R detects the C++ version by simply grepping for CXX_STD in the Makevars file, so even with my fancy ifeq() statement, it still thinks that the package uses C++11.

So now my plan is to simply remove "CXX_STD" from Makevars and "SystemRequirements: C++11" from DESCRIPTION. My understanding is that this will cause the package to no longer build on R 3.5, but as of R 3.6.2, C++11 became the default, so it should build on that version of R or higher.

So just have to remove that from SysReqs :+1:

mpadge commented 1 year ago

https://github.com/ropensci/osmdata/releases/tag/v0.2.0

mpadge commented 1 year ago

Got a CRAN email today that pkg now fails valgrind tests. Need to re-submit by 2nd March to keep pkg on CRAN.

jmaspons commented 1 year ago

Ouch, I think I can't help here. Try to merge #314 before the next submit

mpadge commented 1 year ago

I've got an idea what the problem might be. Should be pretty easy to fix

mpadge commented 1 year ago

It happens with this test code: https://github.com/ropensci/osmdata/blob/c72255544dacd59d9f56a62e75caef1dc372c017/tests/testthat/test-data_frame-osm.R#L251-L254 where doc is: https://github.com/ropensci/osmdata/blob/c72255544dacd59d9f56a62e75caef1dc372c017/tests/testthat/test-data_frame-osm.R#L247-L248 so has adiff data, but passed to osmdata_data_frame without specifying adiff = TRUE or out = "meta".

mpadge commented 1 year ago

That commit fixes the "unitialised values" memcheck errors, but there appear to still be some possible memory leaks...

mpadge commented 1 year ago

The leaks happen here: https://github.com/ropensci/osmdata/blob/906188769c3646bb0fb272f8221850e8c4995793/tests/testthat/test-data_frame-osm.R#L71-L73 because that actually makes the call, and the possible leaks come from curl, not from osmdata. Just have to replace those queries with manually constructed ones, and all should be fine.

mpadge commented 1 year ago

@jmaspons All fixed and good to re-submit. Should hopefully be done today, or tomorrow at latest.

jmaspons commented 1 year ago

Good. Don't forget #314. I can do the merge my self, @mpadge

mpadge commented 1 year ago

That'd be great, thanks! Then i'll still have time to submit tomorrow.