-
Notifications
You must be signed in to change notification settings - Fork 25
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
resolve conflicts for dplyr/tidyr funcs #287
Conversation
thanks @chilampoon ! some tests are failing. Feel free to fix 'ttservice' as well. |
Locally no error now 0 errors ✔ | 1 warning ✖ | 5 notes ✖
Error: R CMD check found WARNINGs
Execution halted I guess others are working on these warnings & notes? 👀 For the deseq2 loading I end up adding those detect & install lines otherwise here test_that("DESeq2 differential trancript abundance - no object",{
res_deseq2 =
test_deseq2_df |>
DESeq2::DESeq() |>
DESeq2::results()
|
@chilampoon please have a look to the failed checks. |
It's 0 errors and 0 warnings on my local machine now, I don't know why there was a warning |
Interesting, @HelenaLC any idea about this? @chilampoon did you follow 100% the |
Currently the tests are failing because of dependencies, some packages are required to install to finish R CMC CHECK. I think it's better to revise and modify all documentations in the package. I can do it if this task hasn't been assigned. Also if @HelenaLC could review a little that would be helpful, thanks! |
Apologies, I tried a newer .github action and screwed the CHECKS, I reverted back. Please pull the new master, which now has the old .github workflow, which fails only for linux, but works for the others. |
library(DESeq2) | ||
library(stringr) | ||
|
||
if (find.package("DESeq2", quiet = TRUE) %>% length %>% equals(0)) { | ||
if (!requireNamespace("BiocManager", quietly = TRUE)) | ||
install.packages("BiocManager", repos = "https://cloud.r-project.org") | ||
BiocManager::install("DESeq2", ask = FALSE) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add this to my PR, so we keep
- drop DESeq2 dependency, and
- resolve conflicts
independent.
I forgot at what stage we are with this |
hi @stemangiola I wonder if anyone is working on the doc/description refactorization or something for tidybulk, if not then I'll probably do it |
( can you please provide this info for the authorship, if you haven't done already? <style type="text/css"></style>
) |
Great, let's first fix the github actions here, and then we merge this PR. We can define the next step after that. I believe the documentation has not been updated yet. Could you please inquire about the existing dedicated issue? |
the issue is still the filter.tidybulk not found warning, which causes the check failure
I don't have this warning locally, I guess it's because of some dependencies problems, plus there are tons of other notes/problems in the report, so I thought it's better to check other scripts as well (not just the dplyr/tidyr functions.R) to resolve the package conflicts.. |
I see, so your strategy would be to also integrate the refactoring of a larger part of the documentation, with the hope that this warning would go away. I think it is worth it, although I am pretty sure this warning is due to an isolated cause. Did you double-check that your documentation for filter mirrors is 100% the one from |
yes will do it step by step, also most of the SingleCellExperiment documents were refactored |
Great, please inform the others on your plans here we don't want to duplicate efforts. If the other have not started yet (re they are unresponsive, feel free to take action). |
Rewrote Roxygen docs for
dplyr_methods.R
andtidyr_methods.R
with the same (updated) format intidySCE
&tidyseurat
.Now the conflicts when loading
tidyomics
become@stemangiola do you want me to update
ttservice
&tidySE
as well..?