Skip to content
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

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

Closed
HenrikBengtsson opened this issue Jan 11, 2023 · 4 comments
Closed

Comments

@HenrikBengtsson
Copy link

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();

civis-r/R/civis_future.R

Lines 84 to 99 in 51c2e22

future <- future::Future(expr = expr,
envir = env,
substitute = substitute,
globals = gp$globals,
packages = unique(c(packages, gp$packages)),
lazy = lazy,
local = local,
gc = gc,
earlySignal = earlySignal,
label = label,
version = "1.8", # see: https://github.com/civisanalytics/civis-r/issues/168
# extra args to scripts_post_containers
required_resources = required_resources,
docker_image_name = docker_image_name,
docker_image_tag = docker_image_tag, ...)

Action

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

@HenrikBengtsson
Copy link
Author

Here is how your package tests will fail:

  • checking tests ...
      Running ‘testthat.R’
     ERROR
    Running the tests in ‘tests/testthat.R’ failed.
    Last 50 lines of output:
       1. └─civis::CivisFuture(quote(2 + 3)) at test_civis_future.R:24:2
       2.   └─future::Future(...)
       3.     └─base (local) dfcn(...)
      ── Error ('test_civis_future.R:61'): run and value work ────────────────────────
      <defunctError/error/condition>
      Error: Argument 'local' is defunct as of future 1.31.0 (2023-??-??)
    ...
      Error: Argument 'local' is defunct as of future 1.31.0 (2023-??-??)
      Backtrace:
          ▆
       1. └─civis::CivisFuture(quote(2 + 2)) at test_civis_future.R:118:2
       2.   └─future::Future(...)
       3.     └─base (local) dfcn(...)
      
      [ FAIL 8 | WARN 4 | SKIP 0 | PASS 1041 ]
      Error: Test failures
      Execution halted
    

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

@HenrikBengtsson
Copy link
Author

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
Copy link

pcooman commented Feb 16, 2023

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
Copy link
Author

It passes my revdep checks. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants