Snitkin-Lab-Umich / prewas

R package to perform data pre-processing for more informative bacterial GWAS
Other
5 stars 3 forks source link

NOTICE: plan(multiprocess) of future is deprecated #13

Open HenrikBengtsson opened 3 years ago

HenrikBengtsson commented 3 years ago

Hi.

This is a friendly reminder that plan(multiprocess) of the future package is deprecated since future 1.20.0 (2020-11-03). It will eventually become defunct and removed. The background for this can be found in https://github.com/HenrikBengtsson/future/issues/420.

Your prewas package relies on multiprocess, cf. https://github.com/Snitkin-Lab-Umich/prewas/search?q=multiprocess.

Please migrate your code to the platform-independent plan(multisession) or the Linux/macOS-specific plan(multicore). If you want to emulate what multiprocess does, you can do something like:

  if (parallelly::supportsMulticore()) {
    oplan <- plan(multicore)
  } else {
    oplan <- plan(multisession)
  }
  on.exit(plan(oplan))

BTW, if you don't already do so, please make sure to undo any plan() you set in your code, as illustrated by the above example. This is needed to guarantee that calling your code won't override settings that the user has set previously. You can read about this in https://future.futureverse.org/reference/plan.html#for-package-developers.

Thank you,

Henrik (maintainer of the future package)

zenalapp commented 2 years ago

@esnitkin914 could you assign this to whoever is going to maintain the package now?

I think the best option is to delete the deprecated code and include documentation of how the user can call any sort of parallelization plan they want in their own script prior to running functions that can be parallelized. Here's an example explanation: http://www.schlosslab.org/mikropml/articles/parallel.html

Once it's updated, the package will also have to be re-submitted to CRAN.

HenrikBengtsson commented 2 years ago

Hello, just a friendly bump on this. It looks like this was fixed in the git repo (https://github.com/Snitkin-Lab-Umich/prewas/commit/0bf01d3c63e8ea0e5306adc9c9620a2702b5329f). What's missing is a CRAN release.

HenrikBengtsson commented 1 year ago

Friendly reminder; 'multiprocess' has been deprecated since 2020-10-30.

HenrikBengtsson commented 1 year ago

Last reminder. Very soon, your package will break and fall of CRAN when I release the updated version of future;

* checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 50 lines of output:
   11.               └─future (local) dfcn(msg = msg, package = .packageName)
   12.                 └─base::.Defunct(...)
  ── Error ('test-reference_alleles.R:202'): parse_snpeff performs as expected when given valid input ──
  <defunctError/error/condition>
  Error: Detected creation of a 'multiprocess' future. Strategy 'multiprocess' is deprecated in future (>= 1.20.0) [2020-10-30]. It will soon become defunct, i.e. produce an error. Instead, specify either 'multisession' (recommended) or 'multicore'.
  Backtrace:
       ▆
    1. └─prewas:::get_ancestral_alleles(tree = temp_tree, mat = temp_dna_list$variant_only_dna_mat) at test-reference_alleles.R:202:2
    2.   ├─base::t(...)
    3.   └─future.apply::future_apply(...)
    4.     └─future.apply::future_lapply(...)
    5.       └─future.apply:::future_xapply(...)
    6.         └─future::future(...)
    7.           ├─future::run(future)
    8.           └─future:::run.Future(future)
    9.             ├─base::do.call(makeFuture, args = args)
   10.             └─future (local) `<FtrStrtg>`(...)
   11.               └─future (local) dfcn(msg = msg, package = .packageName)
   12.                 └─base::.Defunct(...)
  ── Error ('test-reference_alleles.R:258'): parse_snpeff gives error when given invalid input ──
  <defunctError/error/condition>
  Error: Detected creation of a 'multiprocess' future. Strategy 'multiprocess' is deprecated in future (>= 1.20.0) [2020-10-30]. It will soon become defunct, i.e. produce an error. Instead, specify either 'multisession' (recommended) or 'multicore'.
  Backtrace:
       ▆
    1. └─prewas:::get_ancestral_alleles(tree = temp_tree, mat = temp_dna_list$variant_only_dna_mat) at test-reference_alleles.R:258:2
    2.   ├─base::t(...)
    3.   └─future.apply::future_apply(...)
    4.     └─future.apply::future_lapply(...)
    5.       └─future.apply:::future_xapply(...)
    6.         └─future::future(...)
    7.           ├─future::run(future)
    8.           └─future:::run.Future(future)
    9.             ├─base::do.call(makeFuture, args = args)
   10.             └─future (local) `<FtrStrtg>`(...)
   11.               └─future (local) dfcn(msg = msg, package = .packageName)
   12.                 └─base::.Defunct(...)
HenrikBengtsson commented 1 year ago

FYI, future 1.32.0 is now on CRAN making multiprocess is defunct. Your package will begin to produce check errors on CRAN any time soon, and you might get a request to update your package from the CRAN Team.

zenalapp commented 1 year ago

Thanks for letting us know! We're on it