civisanalytics / civis-r

Civis API Client for R: https://civisanalytics.com/products/civis-platform/
Other
16 stars 12 forks source link

Argument 'local' of Future() to become defunct [breaks your package] #244

Closed HenrikBengtsson closed 1 year ago

HenrikBengtsson commented 1 year ago

Background

Argument local for future::Future() is currently defunct for local = FALSE. This argument will soon become completely defunct, i.e. if it is specified, an error is produced.

Issue

R CMD check on your package fails when the above change is effective. This is because, in your CivisFuture(), you are currently passing the default local = TRUE to future::Future();

https://github.com/civisanalytics/civis-r/blob/51c2e2270ed52a977ebbba869a69a76f1a7311cc/R/civis_future.R#L84-L99

Action

Please drop this local argument everywhere. It already now has no effect.

HenrikBengtsson commented 1 year ago

Here is how your package tests will fail:

Source: https://github.com/HenrikBengtsson/future/blob/d70df1c2e18b21a64e11aa341172e9c4e7b5038f/revdep/problems.md?plain=1#L339-L378

HenrikBengtsson commented 1 year ago

To reproduce the above check error with the latest future 1.31.0 on CRAN, set:

export R_FUTURE_CHECK_IGNORE_CIVIS=false

before running R CMD check.

pcooman commented 1 year ago

I submitted an updated version of the civis package (v3.1.0) to CRAN for approval. In this new version, the "local" argument to the CivisFuture() function has been deprecated, and we are no longer passing it on the the future::Future() function. To confirm, I ran R CMD CHECK with R_FUTURE_CHECK_IGNORE_CIVIS set to false and all checks passed.

The new submission passed CRAN's automated checks and is now waiting for manual review, which should be completed within ~5 days. We should be ready on our end for you to make the proposed changes to the future package.

Thank you again for bringing this to our attention and for your patience!

HenrikBengtsson commented 1 year ago

It passes my revdep checks. Thanks.