search

kevinrue / velociraptor

Toolkit for Single-Cell Velocity
https://kevinrue.github.io/velociraptor/
Other
53 stars 10 forks source link

add example for scvelo.params argument #56

Closed ATpoint closed 2 years ago

ATpoint commented 2 years ago

I personally found it unintuitive how to use the scvelo.params argument. I think a little example like this will help others clarifying this.

kevinrue commented 2 years ago

Thanks, and very good point. I'm letting the CI checks run and I'll merge (probably tomorrow in my morning)

kevinrue commented 2 years ago

Good to see one pass. I'm a bit puzzled (and short on time) to investigate the other two. Looks like the code passes fine the Bioconductor daily build. Ugh.

csoneson commented 2 years ago

Thanks @ATpoint - just to expand a bit on the failing runs: I'm also not sure why the mac one fails, but the ubuntu one uses the wrong combination of R and Bioc. I think we need the following configuration in the workflow yml: 

- { os: ubuntu-latest, r: 'devel', bioc: 'devel', cont: "bioconductor/bioconductor_docker:devel", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
- { os: macOS-latest, r: 'devel', bioc: 'devel'}
- { os: windows-latest, r: 'devel', bioc: 'devel'}

Also, could you run devtools::document() and push the .Rd file so that your changes make it into the documentation?

ATpoint commented 2 years ago

Ok, made these changes. Sorry that this little PR makes such a fuzz.

kevinrue commented 2 years ago

Nothing to be sorry about. Typical minor updates to the testing environment that are not directly related to the package functionality. Thanks for your patience :)

kevinrue commented 2 years ago

Quick note to point out that I haven't forgotten about this. I just have one other time sensitive thing to finish first.

This is one of those things that can be fixed in either 5 min or 5 days, trying out different configurations and waiting for CI to run, and I'd rather do it when I can properly focus on the appropriate clean fix.

kevinrue commented 2 years ago

it seems to be working when I push your branch to the main repo, with a little extra commit that @csoneson suggested. Basically just about running devtools::document() See here for the commit history: https://github.com/kevinrue/velociraptor/commits/patch-3

I tried pushing to your branch but apparently edits on your PR are disabled. Can you add that commit yourself? Otherwise I'll have to close your PR and make a new one from my copy of the branch

ATpoint commented 2 years ago

I already pushed the updated Rd files https://github.com/kevinrue/velociraptor/pull/56/commits/860f4bd2c6c0f33ee122aff352a01336ff742264

Anyway, feel free to close and make a PR from your branch :)

kevinrue commented 2 years ago

That's odd, because I pulled your branch and a file still got updated when I ran roxygen. Anyway, yes, I'll do it. Your contributing commits will still make it in the log and be acknowledged :) Thanks!

kevinrue commented 2 years ago

Fixed by #57 Thanks for doing all the leg work @ATpoint !