stemangiola / tidybulk

Brings bulk and pseudobulk transcriptomics to the tidyverse
https://stemangiola.github.io/tidybulk/
164 stars 25 forks source link

resolve conflicts for dplyr/tidyr funcs #287

Closed chilampoon closed 11 months ago

chilampoon commented 1 year ago

Rewrote Roxygen docs for dplyr_methods.R and tidyr_methods.R with the same (updated) format in tidySCE & tidyseurat.

Now the conflicts when loading tidyomics become

> library(tidyomics)
── Attaching core tidyomics packages ────────────────────────────────────────────────────────── tidyomics 0.1.1 ──
✔ dplyr                    1.1.2      ✔ tidyr                    1.3.0 
✔ ggplot2                  3.4.3      ✔ tidyseurat               0.7.2 
✔ nullranges               1.6.2      ✔ tidySingleCellExperiment 1.11.6
✔ plyranges                1.19.0     ✔ tidySummarizedExperiment 1.10.0
✔ tidybulk                 1.11.5     
── Conflicts ──────────────────────────────────────────────────────────────────────────── tidyomics_conflicts() ──
✖ plyranges::between()               masks dplyr::between()
✖ ttservice::bind_cols()             masks tidySummarizedExperiment::bind_cols(), dplyr::bind_cols()
✖ ttservice::bind_rows()             masks tidySummarizedExperiment::bind_rows(), dplyr::bind_rows()
✖ tidySummarizedExperiment::count()  masks matrixStats::count(), dplyr::count()
✖ plyranges::filter()                masks tidySummarizedExperiment::filter(), dplyr::filter(), stats::filter()
✖ plyranges::n()                     masks dplyr::n()
✖ plyranges::n_distinct()            masks dplyr::n_distinct()
✖ tidySummarizedExperiment::rename() masks S4Vectors::rename(), dplyr::rename()
✖ plyranges::slice()                 masks tidySummarizedExperiment::slice(), IRanges::slice(), dplyr::slice()
ℹ Use the conflicted package to force all conflicts to become errors

@stemangiola do you want me to update ttservice & tidySE as well..?

stemangiola commented 1 year ago

thanks @chilampoon ! some tests are failing.

Feel free to fix 'ttservice' as well.

chilampoon commented 1 year ago

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()

DESeq2:: causes errors

stemangiola commented 1 year ago

@chilampoon please have a look to the failed checks.

chilampoon commented 1 year ago

It's 0 errors and 0 warnings on my local machine now, I don't know why there was a warning Warning: Warning: declared S3 method 'filter.tidybulk' not found in github checks...

stemangiola commented 1 year ago

It's 0 errors and 0 warnings on my local machine now, I don't know why there was a warning Warning: Warning: declared S3 method 'filter.tidybulk' not found in github checks...

Interesting, @HelenaLC any idea about this? @chilampoon did you follow 100% the tidySCE example. The solution must be in some difference between the two.

chilampoon commented 1 year ago

It's 0 errors and 0 warnings on my local machine now, I don't know why there was a warning Warning: Warning: declared S3 method 'filter.tidybulk' not found in github checks...

Interesting, @HelenaLC any idea about this? @chilampoon did you follow 100% the tidySCE example. The solution must be in some difference between the two.

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!

stemangiola commented 1 year ago

Currently the tests are failing because of dependencies, some packages are required to install to finish R CMC CHECK.

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.

stemangiola commented 1 year ago

I forgot at what stage we are with this

chilampoon commented 12 months ago

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

stemangiola commented 12 months ago

(

can you please provide this info for the authorship, if you haven't done already?

Email | Institution | First Name | Middle Name(s)/Initial(s) | Last Name | Suffix | Corresponding Author | Home Page URL | Collaborative Group/Consortium | ORCiD -- | -- | -- | -- | -- | -- | -- | -- | -- | -- )
stemangiola commented 12 months ago

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

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?

chilampoon commented 12 months ago

the issue is still the filter.tidybulk not found warning, which causes the check failure

❯ checking S3 generic/method consistency ... WARNING
Warning:   Warning: declared S3 method 'filter.tidybulk' not found
  See section ‘Generic functions and methods’ in the ‘Writing R
  Extensions’ manual.

❯ checking dependencies in R code ... NOTE
  Namespace in Imports field not imported from: ‘pkgconfig’
    All declared Imports should be used.
  Unexported objects imported by ':::' calls:
    ‘glmmSeq:::glmmTMBcore’ ‘glmmSeq:::hyp_matrix’ ‘glmmSeq:::lmer_wald’
    ‘glmmSeq:::organiseStats’
    See the note in ?`:::` about the use of this operator.

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..

stemangiola commented 12 months ago

the issue is still the filter.tidybulk not found warning, which causes the check failure

❯ checking S3 generic/method consistency ... WARNING
Warning:   Warning: declared S3 method 'filter.tidybulk' not found
  See section ‘Generic functions and methods’ in the ‘Writing R
  Extensions’ manual.

❯ checking dependencies in R code ... NOTE
  Namespace in Imports field not imported from: ‘pkgconfig’
    All declared Imports should be used.
  Unexported objects imported by ':::' calls:
    ‘glmmSeq:::glmmTMBcore’ ‘glmmSeq:::hyp_matrix’ ‘glmmSeq:::lmer_wald’
    ‘glmmSeq:::organiseStats’
    See the note in ?`:::` about the use of this operator.

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 SingleCellExperiment?

chilampoon commented 12 months ago

yes will do it step by step, also most of the SingleCellExperiment documents were refactored

stemangiola commented 12 months ago

yes will do it step by step, also most of the SingleCellExperiment documents were refactored

Great, please inform the others on your plans here

https://github.com/stemangiola/tidybulk/issues/282

we don't want to duplicate efforts. If the other have not started yet (re they are unresponsive, feel free to take action).