Improve documentation and package imports #308

william-hutchison · 2024-01-09T06:12:59Z

This pull request picks up on @chilampoon's pull request #297 . In summary:

Polish roxygen2 function documentation (e.g inheriting from other packages when possible)
Improve coding style (e.g "=" converted to "<-")
Switch Github actions workflow from check-bioc to rworkflows
Document function imports in roxygen2, NAMESPACE and DESCRIPTION instead of installing and loading packages within function calls
Update tidyomics ecosystem description in the README and vignette
Replace the example data used in the README

stemangiola

.

stemangiola · 2024-01-09T06:53:19Z

DESCRIPTION

+    AnnotationDbi,
+    DESeq2,
+    Rtsne,
+    Seurat,
+    betareg,
+    boot,
+    broom, 
+    class, 
+    clusterProfiler, 
+    e1071, 
+    edgeR,
+    functional, 
+    glmmSeq, 
+    glmmTMB, 
+    limma, 
+    lme4, 
+    matrixStats,
+    msigdbr, 
+    org.Hs.eg.db,
+    org.Mm.eg.db, 
+    pbapply, 
+    pbmcapply,
+    survival,
+    survminer, 


Thanks for taking this through!

I really think the dependency structure should go back as before. Please try to remove dependency gradually and see if error appear.

Is there any reason to add tidySummarizedExperiment in Suggests?

*please open a new branch for this

stemangiola · 2024-01-09T06:54:37Z

R/cibersort.R

-  # Check if package is installed, otherwise install
-  if (find.package("matrixStats", quiet = TRUE) %>% length %>% equals(0)) {
-    message("tidybulk says: Installing matrixStats needed for cibersort")
-    install.packages("matrixStats", repos = "https://cloud.r-project.org")
-  }
-
  # Eliminate sd == 0


reinstate this would also be part of the effort of dependencies.

stemangiola · 2024-01-12T04:23:50Z

I can help address errors in the checks over the weekend if they haven't been taken care of..

Thanks! Please do. And please coordinate with @william-hutchison. The resubmission is very imminent.

william-hutchison · 2024-01-12T06:15:12Z

Great, thanks for your help @chilampoon! Feel free to continue from here: https://github.com/william-hutchison/tidybulk/tree/add-package-installation-messages

The import system is a bit unconventional in that we are prompting the user to install and load large database packages through the function check_package_availability instead of installing them all automatically. This is working well everywhere except in the unit tests, where the packages must be explicitly installed and loaded (you can see an example of this at the top of the test-bulk_methods_SummarizedExperiment.R file.

So, part of what may be required is to manually install and load missing packages in this way for the unit tests to run.

I also saw multiple unit tests begin to fail after the merge at commit 989009a which will probably be the main challenge to solve.

stemangiola · 2024-02-06T23:41:08Z

@william-hutchison how are we going with this PR?

We have a lot of PR open and we should try to complete them for this Bioc release.

william-hutchison · 2024-02-07T04:44:18Z

Hi @stemangiola, yes I agree. @chilampoon, are you still planning on having a go at fixing the remaining few errors? If not I will try to find some time to finish this off over the next week or so.

stemangiola · 2024-02-07T04:48:55Z

Hi @stemangiola, yes I agree. @chilampoon, are you still planning on having a go at fixing the remaining few errors? If not I will try to find some time to finish this off over the next week or so.

@chilampoon, if you could push this true, it would be amazing. Please, let us know because we are getting the repository ready for the next Bioconductor release..

chilampoon · 2024-02-07T05:03:28Z

sure thing. When I was looking at the errors, I found some of them are not easy to fix although seem trivial. I can fix those relatively easier-to-fix errors by the end of this week and @william-hutchison to fix the rest of them? your understanding of the underlying architecture and unit tests should be much better than mine..

william-hutchison · 2024-02-07T22:46:18Z

Thanks @chilampoon! Yep sounds like a plan, see how you go and I will attempt to fix whatever remains.

chilampoon · 2024-02-11T06:44:32Z

Thanks @chilampoon! Yep sounds like a plan, see how you go and I will attempt to fix whatever remains.

hi @william-hutchison can you add me as a contributor to your tidybulk repo? thx

stemangiola · 2024-02-11T08:56:32Z

Thanks @chilampoon! Yep sounds like a plan, see how you go and I will attempt to fix whatever remains.

hi @william-hutchison can you add me as a contributor to your tidybulk repo? thx

Hello @chilampoon we interact with forks for the tidyomics. Is much better to track who has contributed to what and see the repo history.

chilampoon · 2024-02-11T14:38:04Z

Thanks @chilampoon! Yep sounds like a plan, see how you go and I will attempt to fix whatever remains.

hi @william-hutchison can you add me as a contributor to your tidybulk repo? thx

Hello @chilampoon we interact with forks for the tidyomics. Is much better to track who has contributed to what and see the repo history.

I meant his forked tidybulk repo so I can directly add commits to this branch

william-hutchison · 2024-02-11T22:45:15Z

@chilampoon, sounds good. I have added you as a collaborator to my fork.

stemangiola · 2024-02-12T01:57:53Z

@william-hutchison please update all forks from my master (I have solved the ERROR in the tests).

…rove-documentation

stemangiola · 2024-02-12T02:56:52Z

@william-hutchison please update all forks from my master (I have solved the ERROR in the tests).

Pleaser @chilampoon pull again from my master. I have fixed a bug.

chilampoon · 2024-02-12T03:06:22Z

@william-hutchison please update all forks from my master (I have solved the ERROR in the tests).

Pleaser @chilampoon pull again from my master. I have fixed a bug.

it's in this commit Merge remote-tracking branch 'stemangiola/master' into ...

stemangiola · 2024-02-12T03:21:51Z

@william-hutchison please update all forks from my master (I have solved the ERROR in the tests).

Pleaser @chilampoon pull again from my master. I have fixed a bug.

it's in this commit Merge remote-tracking branch 'stemangiola/master' into ...

Needs to be done again :)

stemangiola · 2024-02-12T06:53:21Z

DESCRIPTION

+    AnnotationDbi,
+    DESeq2,
+    Rtsne,
+    Seurat,
+    betareg,
+    boot,
+    broom, 
+    class, 
+    clusterProfiler, 
+    e1071, 
+    edgeR,
+    functional, 
+    glmmSeq, 
+    glmmTMB, 
+    limma, 
+    lme4, 
+    matrixStats,
+    msigdbr, 
+    org.Hs.eg.db,
+    org.Mm.eg.db, 
+    pbapply, 
+    pbmcapply,
+    survival,
+    survminer, 


@chilampoon and @william-hutchison we need to drop all these new Imports. If Bioconductor does not accept on-the-fly installation anymore, we rather print the message to install if the packages are not installed, and then use

<package>::<function>

but without adding those dependendies to @importFrom, not in DESCRIPTION as Imports

yead I agree on too many Imports, however directly add <package>::<function> inside a function without specifying `@importFrom' will trigger a warning saying '::' or ':::' imports not declared from: xyz packages...

stemangiola · 2024-02-12T06:54:56Z

R/ggplot.R

 #' @importFrom scales extended_breaks
-#' @importFrom stats qlogis plogis
+#' @importFrom functional Compose
 #'


We would not add any dependency here, but check if functional is installed, and if not , error the user, asking for installing.

This should be applied to all cases.

stemangiola · 2024-02-12T06:55:09Z

R/glmmSeq.R

+#' 
+#' @import lme4
+#' @importFrom glmmTMB glmmTMBControl
+#' @importFrom parallel makeCluster
+#' @importFrom parallel clusterExport
+#' @importFrom pbapply pblapply
+#' @importFrom pbmcapply pbmclapply
 #'


stemangiola · 2024-02-12T06:55:23Z

R/utilities.R

+#' @importFrom betareg betareg
+#' @importFrom broom tidy
+#' @importFrom boot logit
+#' @importFrom survival coxph
+#' @importFrom survminer surv_fit
+#' @importFrom survminer ggsurvplot
 multivariable_differential_tissue_composition = function(


stemangiola · 2024-03-06T11:06:05Z

Hello @chilampoon, I hope you're well. Any news with this project? We have put a lot of energies, it would be worth to take it to completion.

chilampoon · 2024-03-10T03:59:42Z

Hello @chilampoon, I hope you're well. Any news with this project? We have put a lot of energies, it would be worth to take it to completion.

yes lets get it done. Re the package dependencies - so many functionalities are included in this package, I wonder if it's possible to remove some of the non-core functions, especially those for running very downstream tasks? instead we could just keep some examples in the vignette, then the package would be more lightweight

stemangiola · 2024-03-10T23:02:57Z

Hello @chilampoon, I hope you're well. Any news with this project? We have put a lot of energies, it would be worth to take it to completion.

yes lets get it done. Re the package dependencies - so many functionalities are included in this package, I wonder if it's possible to remove some of the non-core functions, especially those for running very downstream tasks? instead we could just keep some examples in the vignette, then the package would be more lightweight

Sure we can have this discussion. However better we do one thing at the time.

@chilampoon I see the problem here i that the Github do not pass. I think we are using a not efficient way to approach this problem. Are you doing devtools::check() locally before pushing to the GiHub?

We might be close, but I feel we are stuck on this loop. Do you feel you would like a brief meeting on the best testing checking strategy?

or @william-hutchison do you feel it would be easier for you to bring this PR to completion?

chilampoon · 2024-03-11T18:06:40Z

Yes I run devtools::check() and R CMD CHECK before committing, there are still errors/warnings I haven't addressed.
Sure I am available to meet on Thursday & Friday, my time is EDT @stemangiola

stemangiola · 2024-05-15T13:49:47Z

Hello @chilampoon, sorry for the delay in grant writing time. Any chance we could solve this offline? @william-hutchison, maybe you could advise on why checks are still failing.

chilampoon · 2024-05-16T22:57:35Z

Hello @chilampoon, sorry for the delay in grant writing time. Any chance we could solve this offline? @william-hutchison, maybe you could advise on why checks are still failing.

Hi yeah I have been preparing for my candidacy exam which will happen in two weeks, maybe @william-hutchison could help point out the issues by then? And I can meet on zoom in the second week of June

chilampoon and others added 28 commits September 16, 2023 15:19

resolve tidyomics conflicts and docs clean up

f3bcb06

rm ggplot2 dep and add .Rinstignore

bd82a3c

Merge remote-tracking branch 'upstream/master'

4d8a45b

try to find the warning source

89b106f

update R ver and check-bioc

d7690dd

imports conflict

8709db9

set _R_CHECK_FORCE_SUGGESTS_ to false

b6fdbc4

add pkgconfig

0c1670a

Merge branch 'master' into master

1a0c713

Set automatic Bioconductor and R build-check versions

c564934

Replace check-bioc with rworkflows workflow file

243532c

Update workflow branch triggers

25c605d

Update function imports

bfe5ac8

Update EPIC and EGSEA function imports

c98891b

Fix vignette error

5188581

Remove unneeded files

02dfccc

Update suggested packages

50a94ab

Replace README example data

a162b8e

Update README tidyomics ecosystem links

d8862e2

Version up

225c005

Remove EGSEA import

eea34de

Version up

d59b0a4

Remove unneeded whole package imports

0f60ddd

Remove unneeded whole dplyr import

fd170a8

Replace EPIC import with message

9a9542e

Version up

9baac9c

Update README

7af3822

Version up

2193a71

stemangiola requested changes Jan 9, 2024

View reviewed changes

Merge branch 'master' into chilampoon-improve-documentation

989009a

Merge branch 'master' into chilampoon-improve-documentation

c54b441

chilampoon added 2 commits February 11, 2024 21:13

Merge remote-tracking branch 'stemangiola/master' into chilampoon-imp…

6671ed2

…rove-documentation

resovle line char notes and some warnings

e174749

chilampoon and others added 5 commits February 12, 2024 00:06

resolve doc warnings

fba863a

merge try 1

89c2244

Merge branch 'master' into chilampoon-improve-documentation

59862f2

replace one_of in functions.R

2d2b508

resolve doc warnings and pull udpates

ea0072b

stemangiola requested changes Feb 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve documentation and package imports #308

Improve documentation and package imports #308

william-hutchison commented Jan 9, 2024

stemangiola left a comment •

edited

Loading

stemangiola Jan 9, 2024

stemangiola Jan 9, 2024

stemangiola commented Jan 12, 2024

william-hutchison commented Jan 12, 2024

stemangiola commented Feb 6, 2024

william-hutchison commented Feb 7, 2024

stemangiola commented Feb 7, 2024

chilampoon commented Feb 7, 2024

william-hutchison commented Feb 7, 2024

chilampoon commented Feb 11, 2024

stemangiola commented Feb 11, 2024

chilampoon commented Feb 11, 2024

william-hutchison commented Feb 11, 2024

stemangiola commented Feb 12, 2024

stemangiola commented Feb 12, 2024

chilampoon commented Feb 12, 2024 •

edited

Loading

stemangiola commented Feb 12, 2024

stemangiola Feb 12, 2024

chilampoon Feb 13, 2024

stemangiola Feb 12, 2024

stemangiola Feb 12, 2024

stemangiola Feb 12, 2024

stemangiola commented Mar 6, 2024 •

edited

Loading

chilampoon commented Mar 10, 2024 •

edited

Loading

stemangiola commented Mar 10, 2024 •

edited

Loading

chilampoon commented Mar 11, 2024

stemangiola commented May 15, 2024

chilampoon commented May 16, 2024 •

edited

Loading

Improve documentation and package imports #308

Are you sure you want to change the base?

Improve documentation and package imports #308

Conversation

william-hutchison commented Jan 9, 2024

stemangiola left a comment • edited Loading

Choose a reason for hiding this comment

stemangiola Jan 9, 2024

Choose a reason for hiding this comment

stemangiola Jan 9, 2024

Choose a reason for hiding this comment

stemangiola commented Jan 12, 2024

william-hutchison commented Jan 12, 2024

stemangiola commented Feb 6, 2024

william-hutchison commented Feb 7, 2024

stemangiola commented Feb 7, 2024

chilampoon commented Feb 7, 2024

william-hutchison commented Feb 7, 2024

chilampoon commented Feb 11, 2024

stemangiola commented Feb 11, 2024

chilampoon commented Feb 11, 2024

william-hutchison commented Feb 11, 2024

stemangiola commented Feb 12, 2024

stemangiola commented Feb 12, 2024

chilampoon commented Feb 12, 2024 • edited Loading

stemangiola commented Feb 12, 2024

stemangiola Feb 12, 2024

Choose a reason for hiding this comment

chilampoon Feb 13, 2024

Choose a reason for hiding this comment

stemangiola Feb 12, 2024

Choose a reason for hiding this comment

stemangiola Feb 12, 2024

Choose a reason for hiding this comment

stemangiola Feb 12, 2024

Choose a reason for hiding this comment

stemangiola commented Mar 6, 2024 • edited Loading

chilampoon commented Mar 10, 2024 • edited Loading

stemangiola commented Mar 10, 2024 • edited Loading

chilampoon commented Mar 11, 2024

stemangiola commented May 15, 2024

chilampoon commented May 16, 2024 • edited Loading

stemangiola left a comment •

edited

Loading

chilampoon commented Feb 12, 2024 •

edited

Loading

stemangiola commented Mar 6, 2024 •

edited

Loading

chilampoon commented Mar 10, 2024 •

edited

Loading

stemangiola commented Mar 10, 2024 •

edited

Loading

chilampoon commented May 16, 2024 •

edited

Loading