quanteda / quanteda.textmodels

Text scaling and classification models for quanteda
42 stars 6 forks source link

expect_equal tests need fixing for CRAN r_devel #41

Closed kbenoit closed 3 years ago

kbenoit commented 3 years ago

Received 9 December 2020:

Hi,

Thank you so much for notifying this and explaining the details! I was wondering why the tests on R-devel kept failing these days.

Let me add one more information for testthat users. Testthat released "3rd edition" a while ago. If you turn on the edition, expect_equal() will compare environments by default, and it provides no option to ignore environments in general (as the article below says, you can ignore environments if the object is a formula or a function, but it's not always the case).

expect_equal() previously ignored the environments of formulas and functions. This is most like to arise if you are testing models. It’s worth thinking about what the correct values should be, but if that is to annoying you can opt out of the comparison with ignore_function_env or ignore_formula_env. (https://testthat.r-lib.org/articles/third-edition.html)

As their blog post 1 says testthat's 2nd edition will never go away, it's totally fine to choose check.environment=FALSE. But, you might eventually want to migrate to 3rd edition in order to use their new features. So, this might be a good chance for us to reconsider if it really makes sense to compare the environments. If not, maybe we can simplify the tests so that we'll be more ready for the migration.

(Disclaimer: I'm not a developer of testthat and have almost no experience of 3rd edition, so I might be wrong.)

Best, Yutani

2020年12月9日(水) 21:06 Martin Maechler maechler@stat.math.ethz.ch:

Dear maintainers,

This concerns the CRAN packages

afex assertive.base batchtools bayesplot CausalImpact copula data.table datagovsgR dendroTools Deriv distr6 drugCombo dynamichazard dynparam envalysis fabletools fansi faux fgeo.plot formattable fxtract gamboostLSS ggeasy gghighlight ggplot2 ggrepel ggthemes helda jsonlite leaflet Matrix mixAR mlrCPO model4you mrgsolve nofrills paradox parsetools partition partykit pdqr PlackettLuce plumber purrrlyr quanteda.textmodels rater rBiasCorrection readsdr reinforcelearn robustbase rvinecopulib sampleSelection scdhlm shiny.router SpaDES.core spectacles states statgenGxE tvthemes usmap valaddin wbstats xpose

maintained by one of you:

Alain Hauser alhauser@google.com: CausalImpact Alex M Chubaty alex.chubaty@gmail.com: SpaDES.core Andreas Beger adbeger@gmail.com: states Andrew Redd andrew.redd@hsc.utah.edu: parsetools Arne Henningsen arne.henningsen@gmail.com: sampleSelection Barret Schloerke barret@rstudio.com: plumber Bart-Jan van Rossum bart-jan.vanrossum@wur.nl: statgenGxE Benjamin Christoffersen boennecd@gmail.com: dynamichazard Benjamin Guiastrennec guiastrennec@gmail.com: xpose Benjamin Hofner benjamin.hofner@pei.de: gamboostLSS Brodie Gaslam brodie.gaslam@yahoo.com: fansi Clinton Wong cwxy.clinton@gmail.com: datagovsgR Dominik Krzemiński dominik@appsilon.com: shiny.router Eugene Ha eha@posteo.de: nofrills valaddin Evgeni Chasnovski evgeni.chasnovski@gmail.com: pdqr Georgi N. Boshnakov georgi.boshnakov@manchester.ac.uk: mixAR Heather Turner ht@heatherturner.net: PlackettLuce Heidi Seibold heidi@seibold.co: model4you Henrik Singmann singmann@gmail.com: afex Hiroaki Yutani yutani.ini@gmail.com: gghighlight Jair Andrade jair.albert.andrade@gmail.com: readsdr James Pustejovsky jepusto@gmail.com: scdhlm Jeffrey B. Arnold jeffrey.arnold@gmail.com: ggthemes Jeffrey Pullin jeffrey.pullin@gmail.com: rater Jernej Jevsenak jernej.jevsenak@gmail.com: dendroTools Jeroen Ooms jeroen@berkeley.edu: jsonlite Jesse Piburn piburnjo@ornl.gov: wbstats Joe Cheng joe@rstudio.com: leaflet Jonah Gabry jsg2201@columbia.edu: bayesplot Jonathan Carroll rpkg@jcarroll.com.au: ggeasy Kamil Slowikowski kslowikowski@gmail.com: ggrepel Kenneth Benoit kbenoit@lse.ac.uk: quanteda.textmodels Kun Ren ken@renkun.me: formattable Kyle T Baron kyleb@metrumrg.com: mrgsolve Lionel Henry lionel@rstudio.com: purrrlyr Lisa DeBruine debruine@gmail.com: faux Lorenz A. Kapsner lorenz.kapsner@gmail.com: rBiasCorrection Malcolm Barrett malcolmbarrett@gmail.com: partition Markus Dumke markusdumke@gmail.com: reinforcelearn Martin Binder developer.mb706@doublecaret.com: mlrCPO Martin Maechler maechler@stat.math.ethz.ch: copula robustbase Martin Maechler mmaechler+Matrix@gmail.com: Matrix Matt Dowle mattjdowle@gmail.com: data.table Mauro Lepore maurolepore@gmail.com: fgeo.plot Maxim Nazarov maxim.nazarov@openanalytics.eu: drugCombo Michel Lang michellang@gmail.com: batchtools paradox Mitchell O'Hara-Wild mail@mitchelloharawild.com: fabletools Paolo Di Lorenzo paolo@dilorenzo.pl: usmap Pierre Roudier roudierp@landcareresearch.co.nz: spectacles Quay Au quayau@gmail.com: fxtract Raphael Sonabend raphael.sonabend.15@ucl.ac.uk: distr6 Richard Cotton richierocks@gmail.com: assertive.base Robrecht Cannoodt rcannood@gmail.com: dynparam Ryo Nakagawara ryonakagawara@gmail.com: tvthemes Serguei Sokol sokol@insa-toulouse.fr: Deriv Simon Corde simon.corde@hotmail.fr: helda Thomas Lin Pedersen thomas.pedersen@rstudio.com: ggplot2 Thomas Nagler info@vinecopulib.org: rvinecopulib Torsten Hothorn Torsten.Hothorn@R-project.org: partykit Zacharias Steinmetz steinmetz-z@uni-landau.de: envalysis

[Yes I know that I'm also among the maintainers!]

The NEWS file in "R-devel" (the development version of R) now has in 'NEW Features'

> • all.equal(f, g) for functions now by default also compares their
> environment(.)s, notably via new all.equal method for class
> function.  Comparison of nls() fits, e.g., may now need
> all.equal(m1, m2, check.environment=FALSE).

and all your packages need at least one such 'check.environment=FALSE' in (at least one of) your all.equal() calls --- or expect_equal(..) testthat calls.

See this R-devel (mailing list) thread on why we made the change:

https://stat.ethz.ch/pipermail/r-devel/2020-December/080172.html

and please ask for help / questions on R-devel (the mailing list), or possibly me, if necessary.

Note that I've investigated 'tvthemes' package in particular, as its use of testthat ((as usual in such cases, because of all the 'testthat' tricks leading to obfuscation)) gave an error message which was not at all directly pointing to the issue:

There, I've even opened a github issue with the solution (for 'tvthemes') here:

https://github.com/Ryo-N7/tvthemes/issues/15

The "gist" of it is to define a simple wrapper function

For R-devel (aka R 4.1.x), but also works in older R

expect_eqNe <- function(...) expect_equal(..., check.environment=FALSE)

and then -- in a dozen cases in that case -- use

expect_eqNe() instead of expect_equal()

Alternatively, of course, you'd add " , check.environment=FALSE " (without quotes) to your all.equal() / expect_equal() calls.

In cases as above, it's faster to provide the wrapper and use "global replace everything".

Thank you for helping to make R more powerful and useful by maintaining your packages on CRAN!

With best regards, Martin

Martin Maechler ETH Zurich and R Core team