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

restore all the classes on [.describe #130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

romainfrancois
Copy link

@romainfrancois romainfrancois commented May 14, 2020

I believe this caused the package neuropsychology to fail against dplyr 1.0.0 because a data frame with additional class describe is [ and this method only keeps "describe" as the class.

related to tidyverse/dplyr#5240

@@ -528,7 +528,7 @@ print.describe.single <-
object <- '['(unclass(object),i)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps object <- NextMethod() ?

@romainfrancois
Copy link
Author

For reference, here is how neuropsychology fails:

[master*] 71.2 MiB ❯ revdepcheck::revdep_details(, "neuropsychology")
══ Reverse dependency check ═══════════════════════════ neuropsychology 0.5.0 ══

Status: BROKEN

── Still failing

✖ checking dependencies in R code ... NOTE

── Newly failing

✖ checking examples ... ERROR

── Before ──────────────────────────────────────────────────────────────────────
❯ checking dependencies in R code ... NOTE
  Namespaces in Imports field not imported from:
    ‘htmlTable’ ‘lme4’ ‘stringi’
    All declared Imports should be used.

0 errors ✔ | 0 warnings ✔ | 1 note ✖

── After ───────────────────────────────────────────────────────────────────────
❯ checking examples ... ERROR
  Running examples in ‘neuropsychology-Ex.R’ failed
  The error most likely occurred in:

  > ### Name: describe
  > ### Title: Description of dataframes.
  > ### Aliases: describe
  >
  > ### ** Examples
  >
  > require(neuropsychology)
  >
  > df <- personality
  >
  > describe(df)
  Warning: `transmute_()` is deprecated as of dplyr 0.7.0.
  Please use `transmute()` instead.
  See vignette('programming') for more help
  This warning is displayed once every 8 hours.
  Call `lifecycle::last_warnings()` to see where this warning was generated.
  Error in round(df[2:10], 2) :
    non-numeric argument to mathematical function
  Calls: describe
  Execution halted

❯ checking dependencies in R code ... NOTE
  Namespaces in Imports field not imported from:
    ‘htmlTable’ ‘lme4’ ‘stringi’
    All declared Imports should be used.

1 error ✖ | 0 warnings ✔ | 1 note ✖

in relation with the describe function : https://github.com/romainfrancois/neuropsychology.R/blob/master/R/describe.R#L10 that ends up calling dplyr::mutate() which uses [.describe https://github.com/tidyverse/dplyr/blob/master/R/mutate.R#L202

@romainfrancois
Copy link
Author

Unfortunately I haven't been able to test this thoroughly as I was not able to build Hmisc locally.

@romainfrancois
Copy link
Author

We've now merged extra checking in dplyr so that the error becomes:

library(dplyr, warn.conflicts = FALSE)

neuropsychology::personality %>% 
  select(where(is.numeric)) %>% 
  psych::describe() %>% 
  transmute(a = 1)
#> Error: Can't reconstruct data frame.
#> x The `[` method for class <psych/describe/data.frame> must return a data frame.
#> ℹ It returned a <describe>.

Created on 2020-05-20 by the reprex package (v0.3.0)

@harrelfe
Copy link
Owner

Thanks working on this. I would need a formal pull request and testing before I can proceed. Sorry I don't have time soon to work on this myself.

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

Successfully merging this pull request may close these issues.

2 participants