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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

select_if(<logical vector>) #4213

Closed
grabear opened this issue Feb 21, 2019 · 6 comments
Closed

select_if(<logical vector>) #4213

grabear opened this issue Feb 21, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@grabear
Copy link

@grabear grabear commented Feb 21, 2019

Hello all!
馃槂

馃寫 Intro

During a recent Pull Request (vallenderlab/MicrobiomeR#79), I discovered that our TravisCI build was breaking for some reason. After a few hours of testing code, I still couldn't figure it out. So I created a new PR to test the travis build (See vallenderlab/MicrobiomeR#82 for our process of elimination).

馃寭 Troubleshooting Workflow

Via vallenderlab/MicrobiomeR#82 and the same local error that @sdhutchins was having, we discovered that the problem was with dplyr::select_if:

Here are the Travis builds:
Original branch - https://travis-ci.com/vallenderlab/MicrobiomeR/builds/101668726
Debugging branch - https://travis-ci.com/vallenderlab/MicrobiomeR/builds/101679286

Here is the code in context:
https://github.com/vallenderlab/MicrobiomeR/blob/91eb2c1b9bf124b01cdc662722f484f2665b92cf/R/utils.R#L330-L351

Because the code was running on my local machine and not @sdhutchins computer and the travis servers, we had the chance to compare our libraries. vallenderlab/MicrobiomeR#82 has the session info for mine and @sdhutchins local machines. I copied the dependencies from a previously successful travis build and the current breaking build and found the following differences:

package bad version bad date good version good date
ggsignif 0.5.0 2/20/2019 0.4.0 8/3/2017
microbiome 1.5.28 2/20/2019 1.5.27 2/12/2019
phyloseq 1.27.2 2/20/2019 1.25.2 2/12/2019
ggthemes 4.1.0 2/19/2019 4.0.1 8/24/2018
ellipsis 0.1.0 2/19/2019 1 1/1/1900
forcats 0.4.0 2/17/2019 0.3.0 2/19/2018
dplyr 0.8.0.1 2/15/2019 0.7.8 11/10/2018
rotl 3.0.7 2/15/2019 3.0.6 1/20/2019
R6 2.4.0 2/14/2019 2.3.0 10/4/2018
shades NA 1.3.1 2/14/2019

It seems that dplyr updated recently, and that's what was breaking our code.

馃寱 Solution

We started to both begin messing with our DESCRIPTION file along with our .travis.yml file. We tried to set dplyr's version to 0.7.8, but the travis build "force" installed the newest version (0.8.0.1) regardless..

image

So after some digging I changed the travis config file by adding the lines:

install:
- R -e 'devtools::install_deps(dependencies = TRUE, upgrade = "never")'

This seems to fix the builds but only for:

r:
- release
- devel

馃寳 Other Issues

The other R version builds:

r:
- 3.3.3
- 3.3.2
- 3.3.1
- 3.3.0

will fail because they are newly added and I'm assuming it has something to do with the package cache on travisCI. They fail because devtools is not installed.

Additionally it might have something to do with the recent purrr update as well? I'm not sure.

Any help here would be appreciated. I'm not sure if we're doing something wrong in our code, or if we should change something in our Travis build so that it doesnt force install dplyr 0.8.0.1. The current fix doesn't seem like a very good long term solution.

dplyr - @romainfrancois @hadley
purrr/devtools - @lionel @jennybc @jimhester

Sorry to tag so many, but I thought one of you would at least be able to help us. We speant ~7.5 hours yesterday trying to troubleshoot the problem ourselves and got to a good point. But we are still uncomfortable merging our PR. vallenderlab/MicrobiomeR#79

Cheers,
@grabear

@romainfrancois

This comment has been hidden.

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Feb 22, 2019

Here is a stripped down version of the problem:

library(dplyr, warn.conflicts = FALSE)

ids <- "Sepal.Length"
iris %>% 
  select_if(!names(.) %in% ids)
#> Error: Can't create call to non-callable object

This is not how select_if() works, if it used to work with 0.7.8 it was purely accidental. Here are some alternatives:

library(dplyr, warn.conflicts = FALSE)

iris <- head(iris, 3L)
ids <- "Sepal.Length"
iris %>% 
  select(-ids)
#>   Sepal.Width Petal.Length Petal.Width Species
#> 1         3.5          1.4         0.2  setosa
#> 2         3.0          1.4         0.2  setosa
#> 3         3.2          1.3         0.2  setosa

iris %>% 
  select_at(setdiff(names(.), ids))
#>   Sepal.Width Petal.Length Petal.Width Species
#> 1         3.5          1.4         0.2  setosa
#> 2         3.0          1.4         0.2  setosa
#> 3         3.2          1.3         0.2  setosa

@lionel-
Copy link
Member

@lionel- lionel- commented Feb 22, 2019

Actually it was on purpose that you could supply logical vectors instead of a predicate, this is following purrr semantics for map_if().

@romainfrancois romainfrancois added this to the 0.8.1 milestone Feb 22, 2019
@romainfrancois romainfrancois changed the title Dplyr 0.8.0.1 breaks our package and "force" installs on TravisCI. select_if(<logical vector>) Feb 22, 2019
@grabear
Copy link
Author

@grabear grabear commented Feb 22, 2019

Thanks for looking into this. Any ideas on the Travis CI bit? Or should I ask this somewhere else?

@grabear
Copy link
Author

@grabear grabear commented Feb 22, 2019

Thanks for looking into this. Any ideas on the Travis CI bit? Or should I ask this somewhere else?

Here is a thread to address any TracisCI related fixes: https://travis-ci.community/t/travis-build-ignoring-r-package-version-in-description/2431

@lock
Copy link

@lock lock bot commented Sep 1, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Sep 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants