search

kharchenkolab / cacoa

Single-cell Case Control Analysis
46 stars 7 forks source link

Updated documentation #40

Closed evanbiederstedt closed 1 year ago

evanbiederstedt commented 1 year ago

Is this what you meant @rrydbirk ?

Or would you like me to revert your commit?

Let's discuss here and then we can close this PR if needed. I think I'm confused from the email discussion

rrydbirk commented 1 year ago

@evanbiederstedt, this is fine, wasn't sure if you wanted to revert my commit and then do PR. But since I already resolved all conflicts before merging, I'd prefer not to revert :-)

We need to decide on what we do with @examples. Now, I just included necessary functions to produce examples, but this causes CircleCI to fail. Are we going to include an example Cacoa object in the package? If not, almost all examples need to be wrapped in \donttest since it will take too long (5+ secs) to run almost any example.

If we don't include an example Cacoa object, e.g. by using conos::small_panel.preprocessed, we need to include some prep (sample.groups, cell.groups) for this in all examples (tedious).

We could also go for \dontrun for all examples, but I'm not sure that CRAN will like this...

Just my 10 cents. What do you think?

evanbiederstedt commented 1 year ago

Note: Given conflicts above, we'll need to regenerate the docs in /man and use this branch

Are we going to include an example Cacoa object in the package? If not, almost all examples need to be wrapped in \donttest since it will take too long (5+ secs) to run almost any example.

Check how I do this in conos and pagoda2; a small example for tests is pretty useful.

Most of the examples which take too long to run can be deleted upon the initial submission to CRAN; we could add them later

@rrydbirk

rrydbirk commented 1 year ago

@evanbiederstedt I added a small example dataset, but I didn't add tests. Also, for now, I wrapped all examples in \dontrun. I have to move on to other projects again soon, so this is probably what I'll have time for right now.

Since Conos is not in imports, I'm not sure that CircleCI tests will pass (testthat). I'm unsure of best practice. I tried creating a Conos object for examples, but it'll take up too much space, several MBs.

Please advise on how to proceed.

evanbiederstedt commented 1 year ago

I'll put Conos in Suggests for tests

We did this here: https://github.com/kharchenkolab/conos/blob/main/tests/testthat/test_functions.R#L4-L12

And then with a bit of trial and error, we'll get it working

rrydbirk commented 1 year ago

@evanbiederstedt Not sure I follow you. Conos is already in Suggests, but CircleCI still fails.

evanbiederstedt commented 1 year ago

@rrydbirk

https://app.circleci.com/pipelines/github/kharchenkolab/cacoa/120/workflows/3268ba54-d41e-441d-b74c-a09bd20e8f5b/jobs/121

At least for CircleCI, it cannot find conos

We probably need to explicitly install it: https://github.com/kharchenkolab/cacoa/blob/main/.circleci/config.yml#L29-L31

rrydbirk commented 1 year ago

@evanbiederstedt I can't get this to work. Any hints/ideas?

evanbiederstedt commented 1 year ago

@rrydbirk you got it working!

Please feel free to approve and merge